Added displaying edit GUI when in edit mode.
Details
- Reviewers
michal inka ginsu tomek - Commits
- rCOMM0856ba174897: [native] Displaying edit mode
Checked if after clicking 'Edit' on the target message edit mode is turned on.
Checked if the displayed content of the edited message work in different situations:
- if the original message is already edited,
- if the original message has some markdown,
- if the original message is really long/multiline,
- if the original message has replies/quotes,
With already edited content:
On Android:
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
lib/shared/edit-messages-utils.js | ||
---|---|---|
95–102 ↗ | (On Diff #24961) | Wondering about the performance here. The function that is returned from this hook is currently being called as a selector, so potentially it might be often, and we're iterating all the messages of a thread, which might be time consuming. Do you have some ideas how we can improve the performance here? Maybe we can move something to useMemo call? |
native/chat/chat-input-bar.react.js | ||
1019 ↗ | (On Diff #24961) | This call is really misleading. An argument of useSelector should be a selector - a pure function that takes state and returns something. In this case we're calling a hook as a parameter to another hook - this works, but might cause maintainability issues in the future. It's safer to always call hooks in separate lines. To fix the issue you can e.g. move useSelector call into useGetEditedMessage. |
lib/shared/edit-messages-utils.js | ||
---|---|---|
91–114 ↗ | (On Diff #25032) | I'm wondering if we really need this logic. In chat-selectors we're replacing message text with edited content and return ChatMessageItem - can we somehow use that result here? |
114 ↗ | (On Diff #25032) | We should make dependencies as concrete as possible to reduce the number of recomputations. |
native/chat/chat-input-bar.react.js | ||
545 ↗ | (On Diff #25032) | Can we find a different name? e.g. editedMessage? |
550 ↗ | (On Diff #25032) | Do we have to use animated view? |
1048 ↗ | (On Diff #25032) | Maybe editedMessagePreview? |
Fix typos
lib/shared/edit-messages-utils.js | ||
---|---|---|
91–114 ↗ | (On Diff #25032) | Really good point. Thanks for pointing this out! Changed edit state: https://phab.comm.dev/D7414 Now, in the edit state, I store the whole messageInfo instead of just it's id, and then look it's content up again. |
native/chat/chat-input-bar.react.js | ||
550 ↗ | (On Diff #25032) | I used it to have an animation of sliding up the edit view. However, it doesn't work when hiding the component. I can remove it if we want consistent behavior. |
native/chat/chat-input-bar.react.js | ||
---|---|---|
40–45 | These can be merged |