fix: improve channel preview display hooks and long title issue #3002
Conversation
SDK Size
|
| }, [channel, channelMemberOnline, client]); | ||
| const displayPresence = useMemo(() => { | ||
| return getChannelPreviewDisplayPresence(channel, client); | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps |
There was a problem hiding this comment.
We shouldn't need this, why is eslint complaining ? If we do, let's try to fix the hook instead of disabling the rule
There was a problem hiding this comment.
It is for the additional channelMemberOnline dep
There was a problem hiding this comment.
That should not be the case, that one was there before too and it was also the dependency of a hook.
There was a problem hiding this comment.
Yeah, its weird that it didn't complain before
There was a problem hiding this comment.
Regardless, the issue with the dependencies should be fixed; we should not introduce any more of the eslint-disable comments unless we have a very very good reason to do so
| const [latestMessagePreview, setLatestMessagePreview] = useState< | ||
| LatestMessagePreview<StreamChatGenerics> | ||
| >({ | ||
| created_at: '', |
There was a problem hiding this comment.
This change should be for the better (since the default state seems very broken anyway), but please check and make sure that we don't have code in the downstream components that depends on this (since it's being removed).
There was a problem hiding this comment.
Yeah, I don't see any noticeable bug post this change
| ); | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [channelName, currentUserId, characterLimit, numOfMembers]); | ||
| [channelName, characterLimit, currentUserId, members], |
There was a problem hiding this comment.
We probably aren't sure that members will change its reference each time a member is added (for example, it may very well be the fact that it gets mutated rather than copied on write, as we can see in the other hooks).
I would keep both members and numMembers here
There was a problem hiding this comment.
(for the time being at least, until we convert channel to be fully reactive)
The goal of the PR is to improve the channel preview display hooks to use
useMemoinstead of a combination ofuseStateanduseEffectfor the calculation that would take an additional render.Also, the title logic for the channel preview is improved for channels that have long names. This was a UI issue that was easily noticeable and looked bad.
I am open to discussion if anything doesn't seem correct here.