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

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

Merged
merged 1 commit into from May 2, 2024

Conversation

nimishmedatwal
Copy link
Collaborator

@nimishmedatwal nimishmedatwal commented Apr 26, 2024

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
  • 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.

@zulipbot zulipbot added size: XS area: compose (typeahead) Compose typeahead content and design bug design labels Apr 26, 2024
@zulipbot
Copy link
Member

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

@timabbott
Copy link
Sponsor Member

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

@nimishmedatwal
Copy link
Collaborator Author

nimishmedatwal commented Apr 27, 2024

@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.

@alya alya added the integration review Added by maintainers when a PR may be ready for integration. label Apr 30, 2024
@alya
Copy link
Contributor

alya commented Apr 30, 2024

@timabbott Your question above has been addressed.

@timabbott timabbott merged commit d73a5d0 into zulip:main May 2, 2024
7 checks passed
@timabbott
Copy link
Sponsor Member

timabbott commented May 2, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: compose (typeahead) Compose typeahead content and design bug design integration review Added by maintainers when a PR may be ready for integration. size: XS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove text color change on hover in typeaheads (dark theme)
4 participants