zulip icon indicating copy to clipboard operation
zulip copied to clipboard

Compose(typeahead): Remove text color change on hover.

Open nimishmedatwal opened this issue 10 months ago • 4 comments

Removes hover effect on compose typeahead which causes it to change text color in dark mode.

Fixes: #29842.

Screenshots and screen captures:

Before After
image image
Light Mode Dark Mode
image image
Self-review checklist
  • [x] Self-reviewed the changes for clarity and maintainability (variable names, code reuse, readability, etc.).

Communicate decisions, questions, and potential concerns.

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

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

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

Completed manual review and testing of the following:

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

nimishmedatwal avatar Apr 26 '24 17:04 nimishmedatwal

Hello @zulip/design members, this pull request was labeled with the "design" label, so you may want to check it out!

zulipbot avatar Apr 26 '24 17:04 zulipbot

What CSS generates the hover effect that you're overriding here? And does this bug affect the light theme?

timabbott avatar Apr 26 '24 23:04 timabbott

@timabbott

What CSS generates the hover effect that you're overriding here?

This part of code in dark_theme.css generates the hover effect (line 29).

%dark-theme {
    & a:hover {
        color: hsl(200deg 79% 66%);

        & code {
            color: hsl(200deg 79% 66%);
        }
    }

And does this bug affect the light theme?

No, it does not affect the light theme.

nimishmedatwal avatar Apr 27 '24 11:04 nimishmedatwal

@timabbott Your question above has been addressed.

alya avatar Apr 30 '24 23:04 alya

OK. Merged, thanks @nimishmedatwal, though @karlstolley FYI -- that base a tag CSS is probably a thing we should try to move to a CSS variable to reduce its precedence; didn't want to block this on it because it could be thorny, and doesn't seem urgent, but figured the data point would be of interest.

timabbott avatar May 02 '24 20:05 timabbott