This diff makes it possible to render a robotext that corresponds to a list of messages.
We don't actually render a list of messages anywhere yet.
Depends on D13685
Details
Details
Tested in combination with the following diffs, where I construct RobotextChatMessageInfoItem with an array of messageInfos to represent combined messages (ENG-9559)
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| lib/shared/edit-messages-utils.js | ||
|---|---|---|
| 103 | We made it optional here because for the first time, some messages aren't targetable (merged messages can't be targeted) | |
| lib/shared/reaction-utils.js | ||
| 79 | We made it optional here because for the first time, some messages aren't targetable (merged messages can't be targeted) | |
| 82–84 | This stuff only matters when the message is targetable anyways, since we will return false if not | |
| lib/shared/sidebar-utils.js | ||
| 200 | We made it optional here because for the first time, some messages aren't targetable (merged messages can't be targeted) | |
| 203–205 | This stuff only matters when the message is targetable anyways, since we will return false if not | |
| lib/utils/message-pinning-utils.js | ||
| 30 | We made it optional here because for the first time, some messages aren't targetable (merged messages can't be targeted) | |
| native/chat/utils.js | ||
| 156–159 | We only need to show usernames for composable messages. Robotext messages don't show usernames | |
| native/search/message-search.react.js | ||
| 229–249 | Message search currently only returns text messages, but I added this code here for future-proofing | |
| web/components/message-result.react.js | ||
| 40 | We only need to show usernames for composable messages. Robotext messages don't show usernames | |
| web/selectors/thread-selectors.js | ||
| 67 | We made it optional here because for the first time, some messages aren't targetable (merged messages can't be targeted) | |
| web/tooltips/tooltip-action-utils.js | ||
| 187–194 | We only care about text messages for replies, which are always composable and never combined | |
| 226–232 | We only care about text messages for copies, which are always composable and never combined | |
| lib/shared/chat-message-item-utils.js | ||
|---|---|---|
| 46 | I think it is risky to use a separator that can be already a part of the message key - we're using the same character to separate the keyserver ID from the message ID. Can we use a different separator? Should we also sort the keys? | |
| 58–72 | I think there are a couple of edge cases that should be handled:
So basically, both options are possible:
And, after combining scenarios 1 and 2 it is possible that
All of these can be handled by making sure we're collapsing only messages that don't have reactions / sidebars. Just wanted to highlight the possible issues. | |
| lib/shared/chat-message-item-utils.js | ||
|---|---|---|
| 46 |
No problem, I'll find a better one.
chat-selectors.js calls mergeIntoPrecedingRobotextMessageItem on the messages in the order to oldest to newest. mergeIntoPrecedingRobotextMessageItem is then responsible for maintaing an expected / consistent order. I think it's okay to skip sorting here. | |
| 58–72 |
I had the same thought, and made sure we don't collapse messages with reactions / sidebars in D13687. | |