Fix outstanding UX issues with replies/mentions/keyword notifs #28270

taffyko · 2024-10-22T21:46:53Z

This is a continuation of PR #85 from the now-incorporated matrix-react-sdk, see the discussion there.

Checklist

Tests written for new code (and old code if feasible).
New or updated public/exported symbols have accurate TSDoc documentation.
Linter and other CI checks pass.
I have licensed the changes to Element by completing the Contributor License Agreement (CLA)

Purpose

Closes the long-outstanding issue It's confusing that Replies to your own messages (and highlights in general?) are entirely coloured red. element-meta#744,
@robintown's original PR for this was blocked due to a small edge-case in visual clarity:
When following a link to a message with keyword notifications, there would no longer be any visual indication that the message triggered a notification.
- This PR implements her recommended solution, clarifying notification keywords by displaying them in pills.
- By implementing keyword pills, this closes Notifications triggered by keywords don't decorate messages - Highlight keywords in a pill #8821 as well.

For the "Modern" layout, a bright stripe has been added to the left edge of the highlighted-message background to make mentions/notifications more visible, in the spirit of element-hq/element-meta#1476.
EDIT: This addition has been dropped from this PR, as it needs additional work to adhere to the Compound design system guidelines — I believe it would be best for me to address it in a follow-up PR instead.
For now, disregard the highlighted left edge that appears in the "After" screenshots below.

Before	After

Fixes element-hq/element-meta#744
Fixes #8821

I foresee this change being made across the codebase shortly and want to proactively prevent my PR from falling behind

CLAassistant · 2024-10-22T21:46:59Z

All committers have signed the CLA.

res/css/views/rooms/_EventTile.pcss

res/themes/dark/css/_dark.pcss

src/components/views/messages/TextualBody.tsx

It is clear that it would be best for me to address this piece in a separate PR.

richvdh · 2025-01-09T10:12:51Z

@dbkr's comparison with EX:

username pills are green
the rest of the message is not highlighted
keywords aren't currently highlighted at all

richvdh · 2025-01-09T10:13:11Z

@t3chguy will take this to the design team for guidance

dbkr · 2025-01-09T10:15:44Z

For reference, this is what mention and keywords look like in EX iOS:

t3chguy · 2025-01-13T14:51:26Z

I have requested input from @element-hq/design but did not get anywhere.

t3chguy · 2025-01-17T15:25:49Z

Hey @taffyko - I managed to get some review from the @element-hq/design team. They are happy to land this but would like the font colour for pills to also be updated to --cpd-color-text-on-solid-primary at the same time. Are you able to get this done? Thanks

taffyko · 2025-01-20T18:42:45Z

Hey @taffyko - I managed to get some review from the @element-hq/design team. They are happy to land this but would like the font colour for pills to also be updated to --cpd-color-text-on-solid-primary at the same time. Are you able to get this done? Thanks

On it!

taffyko · 2025-01-20T19:20:40Z

@t3chguy
I've updated with latest develop and tested.
Pills of all types are are already using --cpd-color-text-on-solid-primary for their text color (dark mode and light mode alike), I believe we're all good here!

gaelledel

Thanks very much @taffyko

t3chguy

Looks sane to me, thanks!

t3chguy · 2025-01-22T21:42:21Z

src/components/views/messages/TextualBody.tsx

+    private regExpForKeywordPattern(pattern: string): RegExp {
+        // Reflects the push notification pattern-matching implementation at
+        // https://github.com/matrix-org/matrix-js-sdk/blob/dbd7d26968b94700827bac525c39afff2c198e61/src/pushprocessor.ts#L570
+        return new RegExp("(^|\\W)(" + globToRegexp(pattern) + ")(\\W|$)", "i");
+    }


It'd be nice to reuse the cache in the js-sdk for these regexes but not going to block this change on that

clokep · 2025-01-22T23:40:20Z

Very excited to see this! Thanks for taking the time to do it!

taffyko added 2 commits October 22, 2024 13:44

Fix outstanding UX issues with replies/mentions/keyword notifs

3541391

Use createRoot instead of deprecated ReactDOM.render

16ad814

I foresee this change being made across the codebase shortly and want to proactively prevent my PR from falling behind

taffyko requested a review from a team as a code owner October 22, 2024 21:46

taffyko requested review from t3chguy and robintown October 22, 2024 21:46

github-actions bot added the Z-Community-PR Issue is solved by a community member's PR label Oct 22, 2024

taffyko mentioned this pull request Oct 22, 2024

It's confusing that Replies to your own messages (and highlights in general?) are entirely coloured red. element-hq/element-meta#744

Closed

t3chguy requested a review from a team October 23, 2024 08:08

t3chguy reviewed Oct 23, 2024

View reviewed changes

res/css/views/rooms/_EventTile.pcss Outdated Show resolved Hide resolved

res/themes/dark/css/_dark.pcss Outdated Show resolved Hide resolved

src/components/views/messages/TextualBody.tsx Outdated Show resolved Hide resolved

github-actions bot deployed to Netlify October 23, 2024 08:26 View deployment

github-actions bot deployed to EndToEndTests October 23, 2024 08:26 View deployment

taffyko added 2 commits October 23, 2024 10:11

Clean up react root on unmount

05bd8a2

Remove addition of left-edge highlight on message mentions

6aa1b2e

It is clear that it would be best for me to address this piece in a separate PR.

taffyko requested a review from t3chguy October 23, 2024 18:24

github-actions bot deployed to Netlify October 24, 2024 14:14 View deployment

github-actions bot deployed to EndToEndTests October 24, 2024 14:14 View deployment

Merge branch 'develop' into mention-text-and-pills-ux

3702cc0

Merge branch 'develop' into mention-text-and-pills-ux

7ddfb54

github-actions bot deployed to Netlify January 17, 2025 15:28 View deployment

github-actions bot deployed to EndToEndTests January 17, 2025 15:36 View deployment

taffyko added 2 commits January 20, 2025 10:52

Merge branch 'develop' into mention-text-and-pills-ux

0c24ba3

Update call to ReactRootManager.render

8bdf0c0

t3chguy added the T-Enhancement label Jan 21, 2025

t3chguy removed the request for review from robintown January 21, 2025 08:53

github-actions bot deployed to Netlify January 21, 2025 08:55 View deployment

github-actions bot deployed to EndToEndTests January 21, 2025 09:02 View deployment

gaelledel approved these changes Jan 21, 2025

View reviewed changes

Merge branch 'develop' into mention-text-and-pills-ux

15aeec1

github-actions bot deployed to Netlify January 22, 2025 13:17 View deployment

github-actions bot deployed to EndToEndTests January 22, 2025 13:51 View deployment

Merge branch 'develop' into mention-text-and-pills-ux

6f95f12

github-actions bot deployed to Netlify January 22, 2025 16:50 View deployment

github-actions bot deployed to EndToEndTests January 22, 2025 17:00 View deployment

t3chguy approved these changes Jan 22, 2025

View reviewed changes

t3chguy added this pull request to the merge queue Jan 22, 2025

Merged via the queue into element-hq:develop with commit 68c03db Jan 22, 2025
28 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix outstanding UX issues with replies/mentions/keyword notifs #28270

Fix outstanding UX issues with replies/mentions/keyword notifs #28270

taffyko commented Oct 22, 2024 •

edited by t3chguy

Loading

CLAassistant commented Oct 22, 2024 •

edited

Loading

richvdh commented Jan 9, 2025

richvdh commented Jan 9, 2025

dbkr commented Jan 9, 2025

t3chguy commented Jan 13, 2025

t3chguy commented Jan 17, 2025

taffyko commented Jan 20, 2025

taffyko commented Jan 20, 2025

gaelledel left a comment

t3chguy left a comment

t3chguy Jan 22, 2025

clokep commented Jan 22, 2025

Fix outstanding UX issues with replies/mentions/keyword notifs #28270

Fix outstanding UX issues with replies/mentions/keyword notifs #28270

Conversation

taffyko commented Oct 22, 2024 • edited by t3chguy Loading

Checklist

Purpose

CLAassistant commented Oct 22, 2024 • edited Loading

richvdh commented Jan 9, 2025

richvdh commented Jan 9, 2025

dbkr commented Jan 9, 2025

t3chguy commented Jan 13, 2025

t3chguy commented Jan 17, 2025

taffyko commented Jan 20, 2025

taffyko commented Jan 20, 2025

gaelledel left a comment

Choose a reason for hiding this comment

t3chguy left a comment

Choose a reason for hiding this comment

t3chguy Jan 22, 2025

Choose a reason for hiding this comment

clokep commented Jan 22, 2025

taffyko commented Oct 22, 2024 •

edited by t3chguy

Loading

CLAassistant commented Oct 22, 2024 •

edited

Loading