-
-
Notifications
You must be signed in to change notification settings - Fork 834
Conversation
Fixes: #element-hq/element-web#2284 Signed-off-by: Johannes Marbach <johannesm@element.io>
Should be enough for design review. I won't bother with fixing the tests before. |
This could use screenshots of bubble layout and IRC layout as well |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clearing code review while design/product weigh the feature
Good call @robintown. I missed that the layout differs significantly in these modes. 🤦♂️ Added screenshots above. |
Hi Johennes, thank you for submitting a PR. From a product pov, we don't want to show emotes like normal messages because they are supposed to indicate an action rather than something that a person says so they should be formatted differently. I'm going to close this PR for this reason, but if you would like to offer a fix for the timestamp issue, that would be appreciated. The best way to do that may be to add padding under the profile pic in the timeline. |
Default layout before
Default layout after
Bubbles layout before
Bubbles layout after
IRC layout before
IRC layout after
Advantages
Disadvantages
Fixes: element-hq/element-web#2284
Here's what your changelog entry will look like:
🐛 Bug Fixes
Preview: https://pr8420--matrix-react-sdk.netlify.app
⚠️ Do you trust the author of this PR? Maybe this build will steal your keys or give you malware. Exercise caution. Use test accounts.