MM-15452 - Add ability to override LHS icon for bot accounts #3026
Conversation
utils/utils.jsx
Outdated
| @@ -1339,6 +1340,17 @@ export function imageURLForUser(userIdOrObject) { | |||
| return Client4.getUsersRoute() + '/' + userIdOrObject.id + '/image?_=' + (userIdOrObject.last_picture_update || 0); | |||
| } | |||
|
|
|||
| export function iconImageURLForUser(userIdOrObject) { | |||
| if (typeof userIdOrObject == 'string') { | |||
There was a problem hiding this comment.
nit: It's not an issue here, but can we use === for consistency?
utils/utils.jsx
Outdated
| @@ -1339,6 +1340,17 @@ export function imageURLForUser(userIdOrObject) { | |||
| return Client4.getUsersRoute() + '/' + userIdOrObject.id + '/image?_=' + (userIdOrObject.last_picture_update || 0); | |||
| } | |||
|
|
|||
| export function iconImageURLForUser(userIdOrObject) { | |||
| if (typeof userIdOrObject == 'string') { | |||
| const profile = getUser(store.getState(), userIdOrObject); | |||
There was a problem hiding this comment.
There's still a lot of code that uses the global store, but this is considered deprecated as it introduces potential re-rendering issues (the connector won't trigger a re-render for any state queries that occur here).
Can we refactor this to accept the state object directly? Also, unless this has strong reuse, I might suggest just defining this function in the connector for the one component that uses it. Easy to refactor out later.
There was a problem hiding this comment.
Ripped getting user through getUser out from here since i really didn't to pass in a botId, but rather always a botuser.
| if ((this.props.channelIconUrl && !this.state.svgError) || | ||
| this.props.channelIconUrl !== this.state.channelIconUrl) { | ||
| icon = ( | ||
| <Svg |
There was a problem hiding this comment.
What's the experience on load? Will the icon be empty until we fetch it? Is it feasible to render the default icon until it's loaded?
There was a problem hiding this comment.
Yes, I actually had the default bot showing while loading the image, but the experience was a bit jittery, so it left it as blank for now.
There was a problem hiding this comment.
Can you elaborate on what jittery means in this context? As in, are we flashing between the icons, or flashing no icon at all?
There was a problem hiding this comment.
Jittery in the sense that you momentarily see the default icon which is immediately replaced with the custom icon. Happens really fast. As opposed to No icon and then you see the custom icon appear.
|
@ali-farooq0, one other request to wrap this up: I think the demo plugin uses a bot. Can you update to reflect the override? |
Good idea. Will do. |
lieut-data
added
4: Reviews Complete
amyblais
added
Changelog/Not Needed
Summary
Add ability to override LHS icon for bot accounts
iconImageURLForUserto get user icon imagereact-inlinesvgpackage to support showing SVGs inlineSidebarChannelButtonOrLinkIconcomponent to show user icon if available, otherwise fallback to default bot iconTicket Link
Fixes: https://mattermost.atlassian.net/browse/MM-15452
Related Pull Requests
Preview
Default Icon
Invision Icon
Github Icon
Default Icon
Invision Icon
Github Icon
Updating live (gif)