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

recent_topics: Change background color in dark mode on hover on a row. #30084

Merged
merged 1 commit into from
May 14, 2024

Conversation

shubham-padia
Copy link
Member

@shubham-padia shubham-padia commented May 14, 2024

Fixes #30083.
We have used the dark theme color for the left sidebar row hover: --color-background-active-narrow-filter. We are not using that variable directly since we don't want a change in that color affecting the color in recent conversations.
We've also used a single color on hover compared to 2 different colors for unread and read row before. Light mode also uses a single color on hover.

Relevant CZO convo: link

Screenshots and screen captures:

Hover GIF

recent conversations hover

Self-review checklist
  • Self-reviewed the changes for clarity and maintainability
    (variable names, code reuse, readability, etc.).

Communicate decisions, questions, and potential concerns.

  • Explains differences from previous plans (e.g., issue description).
  • Highlights technical choices and bugs encountered.
  • Calls out remaining decisions and concerns.
  • Automated tests verify logic where appropriate.

Individual commits are ready for review (see commit discipline).

  • Each commit is a coherent idea.
  • Commit message(s) explain reasoning and motivation for changes.

Completed manual review and testing of the following:

  • Visual appearance of the changes.
  • Responsiveness and internationalization.
  • Strings and tooltips.
  • End-to-end functionality of buttons, interactions and flows.
  • Corner cases, error conditions, and easily imagined bugs.

@shubham-padia shubham-padia marked this pull request as ready for review May 14, 2024 05:34
@timabbott
Copy link
Sponsor Member

Hmm, I think it might be better to just use the CSS variable; if we change the color, we'll grep for uses of the variable and ask the question of what our intent is; whereas if we

Also small comment: Let's always cross-link the chat.zulip.org thread for this sort of thing.

@shubham-padia
Copy link
Member Author

Hmm, I think it might be better to just use the CSS variable; if we change the color, we'll grep for uses of the variable and ask the question of what our intent is; whereas if we

I think the sentence is incomplete here 😅

Fixes zulip#30083.
We have used the dark theme color for the left sidebar row hover:
`--color-background-active-narrow-filter`. We are not using that variable
directly since we don't want a change in that color affecting the color in
recent conversations.
We've also used a single color on hover compared to 2 different colors
for unread and read row before. Light mode also uses a single color on
hover.
Relevant CZO conversation:
https://chat.zulip.org/#narrow/stream/101-design/topic/Hover.20color.20in.20Recent.20conversations.20dark.20mode.2E/near/1797727
@timabbott
Copy link
Sponsor Member

Yeah I meant to say "whereas if we just use the same value, we'll probably not ask the question"

@timabbott timabbott merged commit 4ee3724 into zulip:main May 14, 2024
7 checks passed
@timabbott
Copy link
Sponsor Member

Merged, thanks @shubham-padia!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Recent conversations: Hover state on the row looks almost same as non-hover state on the dark mode.
3 participants