Match existing layout (reply button closer to message)... fixes issue knowingly "introduced" in D3306
Details
Details
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Branch
- landmarch8 (branched from master)
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| web/chat/message-action-buttons.js | ||
|---|---|---|
| 174–183 ↗ | (On Diff #9967) | I think we could as well conditionally set a different css class which uses flex-direction: row-reverse. That would reduce the duplication... what do you think? If you think the current approach is cleaner, we can keep it. |
Comment Actions
I think we could as well conditionally set a different css class which uses flex-direction: row-reverse. That would reduce the duplication...
That sounds like a way better/cleaner approach, I’ll update this diff with that change
Comment Actions
address feedback (@palys-swm thanks for suggesting row-reverse, this is a lot cleaner)
Comment Actions
row-reverse results in some weird behavior when selecting text with the cursor, can you test to make sure there are no weird regressions here?
Comment Actions
row-reverse results in some weird behavior when selecting text with the cursor, can you test to make sure there are no weird regressions here?
Ah thanks for flagging that, I'll do some research and then also do some manual testing before landing
edit: I think it's fine for now since we only have icons within this container, but we may need to make adjustments if that changes?
Comment Actions
For me it makes sense. We have a lot more important issue with reverse - when selecting text from a couple of messages. We have a task for it created a long time ago https://www.notion.so/commapp/Handling-multiline-select-on-web-8374bbd134cb424e999edadc4a3deb4c. This one is a lot less important.
| web/chat/message-action-buttons.css | ||
|---|---|---|
| 6 ↗ | (On Diff #10113) | This is a default value, so we could as well remove this class. |
| web/chat/message-action-buttons.css | ||
|---|---|---|
| 6 ↗ | (On Diff #10113) | I'd prefer to keep it to be explicit Particularly in message-action-button: const messageActionButtonsContainer = classNames({
[css.messageActionButtons]: true,
[css.messageActionButtonsViewer]: isViewer,
[css.messageActionButtonsNonViewer]: !isViewer,
});Can change this if there's a strong opinion |
| web/chat/message-action-buttons.css | ||
|---|---|---|
| 6 ↗ | (On Diff #10113) | Agree, there's a value in having this explicitly stated, so we can keep the current approach |