Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keep messages Red, even if mention was removed #24970

Closed
daniellekirkwood opened this issue Mar 27, 2023 · 10 comments · Fixed by matrix-org/matrix-react-sdk#10502
Closed

Keep messages Red, even if mention was removed #24970

daniellekirkwood opened this issue Mar 27, 2023 · 10 comments · Fixed by matrix-org/matrix-react-sdk#10502

Comments

@daniellekirkwood
Copy link
Contributor

daniellekirkwood commented Mar 27, 2023

It's disorienting for users to have a red notification on a room and then to see nothing in the timeline calling their attention... This would happen if the user was mentioned, and then the mention was removed from the timeline message via Edit. In this case, I think the message should stay Red (as it does today)

This is how the message looks when you are mentioned.
Screenshot 2023-03-27 at 14 58 20

This is the suggestion for what the message looks like when the mention is removed but you were originally tagged in it.
Screenshot 2023-03-27 at 14 59 31

We discussed that the all-red messages might be going away soon?
If this is the case, then the "(edit)" should be red, similar to how a keyword would be highlighted in this case.

Originally posted by @daniellekirkwood in #24923 (comment)


Design need:

When a user's mentioned by tag or keyword, what should the message look like? Is it all in red, or not?
When the message is edited to remove the ping, then what should the message look like?

If there is no design given before we come to working on this issue then the plan will be to stylise the "(edited)" text in the message to be red so that users can clearly see the reason that they have a red notification is due to the message origianlly pinging them in some way

@daniellekirkwood
Copy link
Contributor Author

We need a design decision on Direction as this has also been raised: element-hq/element-meta#744

@daniellekirkwood
Copy link
Contributor Author

Found original designs here: https://www.figma.com/file/M15i7l4PSPY1B5M7Zhu80H/Notifications?node-id=2015-16302&t=YgNcioklUd3vSrYO-0

Could we apply that pill styling to the Edited text?

@clokep
Copy link

clokep commented Mar 27, 2023

Maybe also related to #24927 for understanding that you were previously pinged on a message but no longer are.

@rufuskahler
Copy link

@daniellekirkwood – I think the styling the "edited" text (to red) could be a smart solution, especially when users can click to see past edits containing the mention or keyword.

CC @janogarcia for guidance on colour system scalability, @amshakal for guidance on related Notifications patterns and @gaelledel for general web guidance.

@janogarcia
Copy link

Accessibility-wise, relying exclusively on colour to convey status breaks baseline accessibility criteria (Level A).

Color is not used as the only visual means of conveying information, indicating an action, prompting a response, or distinguishing a visual element.

https://www.w3.org/WAI/WCAG21/quickref/?levels=aaa&showtechniques=141#use-of-color

Design systems (and usability) wise, I think we should not rely on red for mentions and think about an alternative decoration, keeping the semantics of red for critical stuff (error states and feedback, destructive actions, etc.).

Usability-wise, I think we should make the UI smarter and remove the mention state everywhere if the mention no longer exists.

So, I agree that it can be disorienting (symptom), but not on how to tackle it, patching it without addressing none of the underlying core issues/debt.

@rufuskahler
Copy link

This makes sense @janogarcia. @daniellekirkwood, if you agree, can we remove the X-Needs-Design label? For prioritisation purposes, I'd also be interested to know what is the occurrence of this issue?

@daniellekirkwood
Copy link
Contributor Author

daniellekirkwood commented Mar 30, 2023

I agree with @janogarcia but we need a design to work off. This was suggested as a short term solution as I'm sure the design system will be addressing all of these things but we need something to improve today's experience.

We can remove the mention state but not the notification so the user will receive a red notification and then have no idea why they got it if we don't mention something in the original message that highlighted it.

This issue is the scope of this work for now. We will review this once better designs are available.

Knowing this is the case; We'll move forwards with the Red "Edited" idea (unless y'all want to prep another design?).

On the a11y front, turning it red is not the only way it catches the user's attention as before being edited the edit button would not have been there so I'm not too worried about that at this stage (but yes, an improvement would be needed).

@rufuskahler 👀

@daniellekirkwood
Copy link
Contributor Author

FYI moved the chat to an internal document here: https://docs.google.com/document/d/1AvHmbGH3XHN6lU-B9xWyMrU0Rv4Ej6DYIsA2ef_9MIs/edit?usp=sharing

@daniellekirkwood
Copy link
Contributor Author

As per the decision made in comments in this document we'd like to turn the "Edited" button text red.

However, @kerryarchibald already has a build ready to commit. @rufuskahler would you review the images or Netlify in the PR linked above and let us know if you think the solution we have already is enough, or if you'd still like to move ahead with changing "Edited" to red along with message text?

Ideally we'd have an answer to this by the end of Wednesday - is that enough time for you?
If we don't hear back, let's go ahead and commit the PR with the grey text, we can always create another issue to address the text colour change in the future.

@daniellekirkwood
Copy link
Contributor Author

discussed with @rufuskahler in DMs and we're going to ask that the text is Red.

So when a user was mentioned, then their tag was removed, it would look like this:
Screenshot 2023-04-04 at 16 28 50

this would also apply to keyword tags

su-ex added a commit to SchildiChat/element-desktop that referenced this issue Apr 25, 2023
* Fixes for [CVE-2023-30609](https://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=CVE-2023-30609) / GHSA-xv83-x443-7rmw
* Pick sensible default option for phone country dropdown ([\#10627](matrix-org/matrix-react-sdk#10627)). Fixes element-hq/element-web#3528.
* Relate field validation tooltip via aria-describedby ([\#10522](matrix-org/matrix-react-sdk#10522)). Fixes element-hq/element-web#24963.
* Handle more completion types in rte autocomplete ([\#10560](matrix-org/matrix-react-sdk#10560)). Contributed by @alunturner.
* Show a tile for an unloaded predecessor room if it has via_servers ([\#10483](matrix-org/matrix-react-sdk#10483)). Contributed by @andybalaam.
* Exclude message timestamps from aria live region ([\#10584](matrix-org/matrix-react-sdk#10584)). Fixes element-hq/element-web#5696.
* Make composer format bar an aria toolbar ([\#10583](matrix-org/matrix-react-sdk#10583)). Fixes element-hq/element-web#11283.
* Improve accessibility of font slider ([\#10473](matrix-org/matrix-react-sdk#10473)). Fixes element-hq/element-web#20168 and element-hq/element-web#24962.
* fix file size display from kB to KB ([\#10561](matrix-org/matrix-react-sdk#10561)). Fixes element-hq/element-web#24866. Contributed by @NSV1991.
* Handle /me in rte ([\#10558](matrix-org/matrix-react-sdk#10558)). Contributed by @alunturner.
* bind html with switch for manage extension setting option ([\#10553](matrix-org/matrix-react-sdk#10553)). Contributed by @NSV1991.
* Handle command completions in RTE ([\#10521](matrix-org/matrix-react-sdk#10521)). Contributed by @alunturner.
* Add room and user avatars to rte ([\#10497](matrix-org/matrix-react-sdk#10497)). Contributed by @alunturner.
* Support for MSC3882 revision 1 ([\#10443](matrix-org/matrix-react-sdk#10443)). Contributed by @hughns.
* Check profiles before starting a DM ([\#10472](matrix-org/matrix-react-sdk#10472)). Fixes element-hq/element-web#24830.
* Quick settings: Change the copy / labels on the options ([\#10427](matrix-org/matrix-react-sdk#10427)). Fixes element-hq/element-web#24522. Contributed by @justjanne.
* Update rte autocomplete styling ([\#10503](matrix-org/matrix-react-sdk#10503)). Contributed by @alunturner.
* Workaround Squirrel.Mac wedging app restart after failed update check ([\element-hq#629](element-hq#629)).
* Fix error about webContents on log out ([\element-hq#627](element-hq#627)).
* Fix error when breadcrumb image fails to load ([\element-hq#609](element-hq#609)).
* Fix create subspace dialog not working ([\#10652](matrix-org/matrix-react-sdk#10652)). Fixes element-hq/element-web#24882
* Fix multiple accessibility defects identified by AXE ([\#10606](matrix-org/matrix-react-sdk#10606)).
* Fix view source from edit history dialog always showing latest event ([\#10626](matrix-org/matrix-react-sdk#10626)). Fixes element-hq/element-web#21859.
* #21451 Fix WebGL disabled error message ([\#10589](matrix-org/matrix-react-sdk#10589)). Contributed by @rashmitpankhania.
* Properly translate errors in `AddThreepid.ts` so they show up translated to the user but not in our logs ([\#10432](matrix-org/matrix-react-sdk#10432)). Contributed by @MadLittleMods.
* Fix overflow on auth pages ([\#10605](matrix-org/matrix-react-sdk#10605)). Fixes element-hq/element-web#19548.
* Fix incorrect avatar background colour when using a custom theme ([\#10598](matrix-org/matrix-react-sdk#10598)). Contributed by @jdauphant.
* Remove dependency on `org.matrix.e2e_cross_signing` unstable feature ([\#10593](matrix-org/matrix-react-sdk#10593)).
* Update setting description to match reality ([\#10600](matrix-org/matrix-react-sdk#10600)). Fixes element-hq/element-web#25106.
* Fix no identity server in help & about settings ([\#10563](matrix-org/matrix-react-sdk#10563)). Fixes element-hq/element-web#25077.
* Fix: Images no longer reserve their space in the timeline correctly ([\#10571](matrix-org/matrix-react-sdk#10571)). Fixes element-hq/element-web#25082. Contributed by @kerryarchibald.
* Fix issues with inhibited accessible focus outlines ([\#10579](matrix-org/matrix-react-sdk#10579)). Fixes element-hq/element-web#19742.
* Fix read receipts falling from sky ([\#10576](matrix-org/matrix-react-sdk#10576)). Fixes element-hq/element-web#25081.
* Fix avatar text issue in rte ([\#10559](matrix-org/matrix-react-sdk#10559)). Contributed by @alunturner.
* fix resizer only work with left mouse click ([\#10546](matrix-org/matrix-react-sdk#10546)). Contributed by @NSV1991.
* Fix send two join requests when joining a room from spotlight search ([\#10534](matrix-org/matrix-react-sdk#10534)). Fixes element-hq/element-web#25054.
* Highlight event when any version triggered a highlight ([\#10502](matrix-org/matrix-react-sdk#10502)). Fixes element-hq/element-web#24923 and element-hq/element-web#24970. Contributed by @kerryarchibald.
* Fix spacing of headings of integration manager on General settings tab ([\#10232](matrix-org/matrix-react-sdk#10232)). Fixes element-hq/element-web#24085. Contributed by @luixxiul.
su-ex added a commit to SchildiChat/element-web that referenced this issue Apr 25, 2023
* Fixes for [CVE-2023-30609](https://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=CVE-2023-30609) / GHSA-xv83-x443-7rmw
* Pick sensible default option for phone country dropdown ([\element-hq#10627](matrix-org/matrix-react-sdk#10627)). Fixes element-hq#3528.
* Relate field validation tooltip via aria-describedby ([\element-hq#10522](matrix-org/matrix-react-sdk#10522)). Fixes element-hq#24963.
* Handle more completion types in rte autocomplete ([\element-hq#10560](matrix-org/matrix-react-sdk#10560)). Contributed by @alunturner.
* Show a tile for an unloaded predecessor room if it has via_servers ([\element-hq#10483](matrix-org/matrix-react-sdk#10483)). Contributed by @andybalaam.
* Exclude message timestamps from aria live region ([\element-hq#10584](matrix-org/matrix-react-sdk#10584)). Fixes element-hq#5696.
* Make composer format bar an aria toolbar ([\element-hq#10583](matrix-org/matrix-react-sdk#10583)). Fixes element-hq#11283.
* Improve accessibility of font slider ([\element-hq#10473](matrix-org/matrix-react-sdk#10473)). Fixes element-hq#20168 and element-hq#24962.
* fix file size display from kB to KB ([\element-hq#10561](matrix-org/matrix-react-sdk#10561)). Fixes element-hq#24866. Contributed by @NSV1991.
* Handle /me in rte ([\element-hq#10558](matrix-org/matrix-react-sdk#10558)). Contributed by @alunturner.
* bind html with switch for manage extension setting option ([\element-hq#10553](matrix-org/matrix-react-sdk#10553)). Contributed by @NSV1991.
* Handle command completions in RTE ([\element-hq#10521](matrix-org/matrix-react-sdk#10521)). Contributed by @alunturner.
* Add room and user avatars to rte ([\element-hq#10497](matrix-org/matrix-react-sdk#10497)). Contributed by @alunturner.
* Support for MSC3882 revision 1 ([\element-hq#10443](matrix-org/matrix-react-sdk#10443)). Contributed by @hughns.
* Check profiles before starting a DM ([\element-hq#10472](matrix-org/matrix-react-sdk#10472)). Fixes element-hq#24830.
* Quick settings: Change the copy / labels on the options ([\element-hq#10427](matrix-org/matrix-react-sdk#10427)). Fixes element-hq#24522. Contributed by @justjanne.
* Update rte autocomplete styling ([\element-hq#10503](matrix-org/matrix-react-sdk#10503)). Contributed by @alunturner.
* Fix create subspace dialog not working ([\element-hq#10652](matrix-org/matrix-react-sdk#10652)). Fixes element-hq#24882
* Fix multiple accessibility defects identified by AXE ([\element-hq#10606](matrix-org/matrix-react-sdk#10606)).
* Fix view source from edit history dialog always showing latest event ([\element-hq#10626](matrix-org/matrix-react-sdk#10626)). Fixes element-hq#21859.
* element-hq#21451 Fix WebGL disabled error message ([\element-hq#10589](matrix-org/matrix-react-sdk#10589)). Contributed by @rashmitpankhania.
* Properly translate errors in `AddThreepid.ts` so they show up translated to the user but not in our logs ([\element-hq#10432](matrix-org/matrix-react-sdk#10432)). Contributed by @MadLittleMods.
* Fix overflow on auth pages ([\element-hq#10605](matrix-org/matrix-react-sdk#10605)). Fixes element-hq#19548.
* Fix incorrect avatar background colour when using a custom theme ([\#10598](matrix-org/matrix-react-sdk#10598)). Contributed by @jdauphant.
* Remove dependency on `org.matrix.e2e_cross_signing` unstable feature ([\element-hq#10593](matrix-org/matrix-react-sdk#10593)).
* Update setting description to match reality ([\element-hq#10600](matrix-org/matrix-react-sdk#10600)). Fixes element-hq#25106.
* Fix no identity server in help & about settings ([\element-hq#10563](matrix-org/matrix-react-sdk#10563)). Fixes element-hq#25077.
* Fix: Images no longer reserve their space in the timeline correctly ([\element-hq#10571](matrix-org/matrix-react-sdk#10571)). Fixes element-hq#25082. Contributed by @kerryarchibald.
* Fix issues with inhibited accessible focus outlines ([\element-hq#10579](matrix-org/matrix-react-sdk#10579)). Fixes element-hq#19742.
* Fix read receipts falling from sky ([\element-hq#10576](matrix-org/matrix-react-sdk#10576)). Fixes element-hq#25081.
* Fix avatar text issue in rte ([\element-hq#10559](matrix-org/matrix-react-sdk#10559)). Contributed by @alunturner.
* fix resizer only work with left mouse click ([\element-hq#10546](matrix-org/matrix-react-sdk#10546)). Contributed by @NSV1991.
* Fix send two join requests when joining a room from spotlight search ([\#10534](matrix-org/matrix-react-sdk#10534)). Fixes element-hq#25054.
* Highlight event when any version triggered a highlight ([\element-hq#10502](matrix-org/matrix-react-sdk#10502)). Fixes element-hq#24923 and element-hq#24970. Contributed by @kerryarchibald.
* Fix spacing of headings of integration manager on General settings tab ([\element-hq#10232](matrix-org/matrix-react-sdk#10232)). Fixes element-hq#24085. Contributed by @luixxiul.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants