Replace (IRC) with flair #1637
Replace (IRC) with flair #1637
Conversation
If a user has public groups that are honoured in their flair, remove the (IRC) to give the appearance that the flair replaces it.
lukebarnard1
force-pushed
the
luke/replace-IRC-with-flair
branch
from
7c8f5f4
to
3addb92
Compare
| onClick: React.PropTypes.func, | ||
| }; | ||
|
|
||
| let groups = this.state.groups || []; |
There was a problem hiding this comment.
This got a bit confusing between the local variables groups, this.state.groups and this.state.relatedGroups. Maybe make the local var displayedGroups and this.state.groups into this.state.userGroups?
There was a problem hiding this comment.
Yep, fair enough
| }); | ||
| } else { | ||
| groups = []; | ||
| } |
There was a problem hiding this comment.
This also feels like it ought to be pulled out to a displayedGroupsForUserInRoom() or something, but not urgent. Mind you, it would still presumably live in this class given it's having to worry about listening to the related groups changes.
There was a problem hiding this comment.
I think it'd still be clearer, so I shall make it so.
| if (this.state.relatedGroups && this.state.relatedGroups.length > 0) { | ||
| groups = groups.filter((groupId) => { | ||
| return this.state.relatedGroups.includes(groupId); | ||
| }); |
There was a problem hiding this comment.
Another (faster) way to do this now we have ES6 would be with Set so you could do an actual intersection.
There was a problem hiding this comment.
Apparently, Sets don't have a native intersection operator/function. You'd end up doing the Set equivalent of the above, which is to filter and use has instead of includes (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Set)
There was a problem hiding this comment.
Ah, javascript. Well, I'd imagine it'd still be faster as you'd expect a membership test to be faster with a set than a list, but probably not enough that it's worth changing.
| <span className="mx_SenderProfile_name"> | ||
| { nameElem } | ||
| </span> | ||
| { this.props.enableFlair ? |
There was a problem hiding this comment.
Should we do the name mangling above if enableFlair is false?
There was a problem hiding this comment.
Nope, I shall change that.
If a user has public groups that are honoured in their flair, remove the (IRC) to give the appearance that the flair replaces it.
This required a total refactor of how is used. I think it makes sense to do the filtering outside of the component itself anyway, but it was necessary to do this so that SenderProfile could strip "(IRC)" accordingly.
Sadly, this meant SenderProfile became stateful.