Details
Please watch the demo video to see the changes I made:
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
When we have a couple of reactions, it is not clear to which of these the number corresponds.
I'm surprised how easy it was to introduce the picker!
| native/navigation/tooltip.react.js | ||
|---|---|---|
| 647–651 ↗ | (On Diff #21549) | Are we sure that we want to render the picker for all the tooltips? Can we render it only for the tooltips where reaction can be made? |
When we have a couple of reactions, it is not clear to which of these the number corresponds.
@tomek could you elaborate on what you mean by the number these correspond to?
Sure! I was talking about the number from the video
The 2 probably corresponds to the 4th emoji, but I think we should make it more obvious, by e.g. increasing the padding between emoji-number pairs, changing the background, or something else. It doesn't need to be addressed here - the main issue here is always rendering the picker.
Sure! I was talking about the number from the video
The 2 probably corresponds to the 4th emoji, but I think we should make it more obvious, by e.g. increasing the padding between emoji-number pairs, changing the background, or something else. It doesn't need to be addressed here - the main issue here is always rendering the picker.
Created a task to track this
| native/chat/multimedia-message-tooltip-button.react.js | ||
|---|---|---|
| 160–164 ↗ | (On Diff #21718) | This change is needed because it is blocking the rerender of this component whenever the value of showEmojiKeyboard changes. Since this component matches the pattern of robotext-message-tooltip-button.react and text-message-tooltip-button.react, I believe that a better approach would be just to memoize the expensive part of the component which is InnerMultimediaMessage |
| native/chat/multimedia-message-tooltip-button.react.js | ||
|---|---|---|
| 160–164 ↗ | (On Diff #21718) | It makes sense to have a consistent approach, but could you explain a little bit more what do you mean by:
Why using memo causes that? Why removing the memo fixes the issue? |
| native/chat/multimedia-message-tooltip-button.react.js | ||
|---|---|---|
| 160–164 ↗ | (On Diff #21718) | From what I understand about shared values, I am mutating the value field of showEmojiKeyboard, but the reference (which I am passing down as a prop) was staying the same so a rerender was not happening. context on shared value objects Please let me know if you have more questions about this |