Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Change the text color of highlighted messages #9199

Closed
wants to merge 1 commit into from

Conversation

robintown
Copy link
Member

@robintown robintown commented Aug 17, 2022

This is a reintroduction of the changes from #5724, since @niquewoodhouse expressed interest in doing a design review, but the author was not available to update the branch.

. Before After
Modern layout Screenshot 2022-08-17 at 08-18-51 Element #element-dev matrix org Screenshot 2022-08-17 at 08-18-33 Element #element-dev matrix org
Bubble layout Screenshot 2022-08-17 at 08-19-08 Element #element-dev matrix org Screenshot 2022-08-17 at 08-17-50 Element #element-dev matrix org

Closes element-hq/element-meta#744

Checklist

  • Linter and other CI checks pass

Here's what your changelog entry will look like:

✨ Features

@robintown robintown added the T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements label Aug 17, 2022
@robintown robintown requested a review from a team as a code owner August 17, 2022 12:25
Copy link
Contributor

@weeman1337 weeman1337 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested the deployed version. LGTM 👍

@nhhollander
Copy link

Thanks for taking this over, I haven't had time to do much side work since I started a full time job. ❤️

@nadonomy
Copy link
Contributor

@niquewoodhouse bump on review/iteration on this

@robintown thx for picking up this PR. How difficult is it to stylise the words or mentions causing the highlight? These designs are old, but should illustrate the desired implementation when we explored this before:

https://www.figma.com/file/M15i7l4PSPY1B5M7Zhu80H/Notifications?node-id=2015%3A16302

Copy link
Contributor

@niquewoodhouse niquewoodhouse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@robintown
Copy link
Member Author

@nadonomy I'm going to attempt to add keyword pills to this PR, hopefully later this week when I have more time

@ara4n
Copy link
Member

ara4n commented Oct 17, 2022

fwiw the intention on highlights was always to only highlight the content that triggered the bing rather than the whole line - the "whole line goes red" was a quick hack that we (I?) did, and then got stuck :(

@ara4n
Copy link
Member

ara4n commented Oct 17, 2022

(removing the highlight colour entirely, as per the current PR, seems like a major step backwards?)

@Avamander
Copy link

Could this be merged?

The red color constantly confuses users and looks seriously ugly. Removing it would be a step forward until the messages are highlighted properly.

@kenwuuu
Copy link
Contributor

kenwuuu commented Mar 30, 2023

@robintown, or anyone else: I don't understand why keyword highlights blocks this from being merged, could I get some clarification?

This doesn't depend on highlights, and highlights don't depend on this. Please publish this PR, this should get this merged in because that's how we make progress, with small incremental changes. Do we need this many reviewers also?

@kenwuuu
Copy link
Contributor

kenwuuu commented Mar 31, 2023

Screen Shot 2023-03-31 at 16 05 35

Looks like replies to replies to your messages have the desired appearance that you guys are looking for? Should help you track down the issue and code a fix.

@robintown
Copy link
Member Author

@kenwuuu, Here's an example that should show why we need to turn keyword highlights into pills if we are to remove the red text color:

Screenshot 2023-04-05 at 15-03-35 Element 7 Matrix HQ

Here I've set up a keyword for "video rooms", which is why the first and second messages have notified me, but because I've clicked on a link to the second message, I can't see the yellow background color. So, there needs to be some secondary indication that this message has caused a highlight.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

It's confusing that Replies to your own messages (and highlights in general?) are entirely coloured red.
9 participants