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.

Sign up for GitHub

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

Jump to bottom

ActionList.Item using wrong colors and get invisible in high-contrast theme #3044

Closed
pablonete opened this issue Mar 16, 2023 · 4 comments · Fixed by primer/view_components#1889
Closed

ActionList.Item using wrong colors and get invisible in high-contrast theme #3044

pablonete opened this issue Mar 16, 2023 · 4 comments · Fixed by primer/view_components#1889
Labels
bug Something isn't working component: ActionList react Stale

Comments

@pablonete
Copy link
Contributor

pablonete commented Mar 16, 2023 •
edited
Loading

Description

We're using a <ActionList.Item variant="danger" ... /> and it renders wrong in high-contrast themes (both dark and light) but not on any other theme:
image
Also wrong on hover:
image

Interestingly, text color gets fixed on resize (e.g. just opening DevTools) while icon color stays always wrong.

https://github.com/github/github/blob/8d9083549de626e8b3fa9667a2de09840f39642f/app/assets/modules/custom-properties/components/DefinitionsTable.tsx#L74

Steps to reproduce

I'll go with the Monolith Storybook where it's easier:

  1. npm run storybook in a github/github codespace
  2. Once the page opens :6006, click Color scheme and use light_high_contrast
  3. Click on "CustomPropertiesSchemaPage"
  4. Click on the ellipsis button at the right to show the actions menu

Expected: label and icon readable
Actual: icon visible and label transparent, the opposite on mouse hover

Some repro videos on slack.

Version

v35.21.0

Browser

Chrome, Firefox
@pablonete pablonete added the bug Something isn't working label Mar 16, 2023
@langermank langermank mentioned this issue Mar 16, 2023
Merged
3 tasks
@langermank
Copy link
Contributor

I've got a fix for this in Rails, but looking at the React code for ActionList I'm not sure how to achieve the same thing. We need to target the hover state.

@langermank langermank reopened this Mar 17, 2023
@siddharthkp
Copy link
Member

siddharthkp commented Mar 20, 2023 •
edited
Loading

Here's the matching react PR for icon and description: #3045

@langermank There are 2 bugs in this issue, keeping this issue open because we still don't know what causes the strange rendering issue

@siddharthkp siddharthkp mentioned this issue Mar 20, 2023
Closed
siddharthkp added a commit that referenced this issue Jul 10, 2023
@siddharthkp
Add fixtures for #3044
8ffcbb2
This was referenced Jul 10, 2023
Merged
Merged
github-merge-queue bot pushed a commit that referenced this issue Jul 10, 2023
@siddharthkp
Add fixtures for #3044 (#3504)
4a70ce3 
* Add fixtures for #3044

* test(vrt): update snapshots

* oops my bad!

---------

Co-authored-by: siddharthkp <siddharthkp@users.noreply.github.com>
github-merge-queue bot pushed a commit that referenced this issue Jul 12, 2023
@siddharthkp
ActionList: Fix icon and description hover color for danger variant (#…
c3f1e3c 
…3505)

* Add fixtures for #3044

* test(vrt): update snapshots

* oops my bad!

* Fix for icon and description

* Create cyan-cycles-provide.md

* update jest snapshots

* test(vrt): update snapshots

* Update cyan-cycles-provide.md

* update snapshots!

* test(vrt): update snapshots

---------

Co-authored-by: siddharthkp <siddharthkp@users.noreply.github.com>
@github-actions
Copy link
Contributor

github-actions bot commented Oct 8, 2023

Hi! This issue has been marked as stale because it has been open with no activity for 180 days. You can comment on the issue or remove the stale label to keep it open. If you do nothing, this issue will be closed in 7 days.

@github-actions github-actions bot added the Stale label Oct 8, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Oct 15, 2023
@siddharthkp
Copy link
Member

This was fixed in #3505 and released in v35.27.0!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working component: ActionList react Stale
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Bug fix: ActionList danger variant hover/active contrast primer/view_components
5 participants
@pablonete @siddharthkp @joshblack @langermank @primer-css