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

Focus outline clean up and only show on focus:visible #9093

Open
7 tasks
alex-page opened this issue Apr 25, 2023 · 10 comments
Open
7 tasks

Focus outline clean up and only show on focus:visible #9093

alex-page opened this issue Apr 25, 2023 · 10 comments
Assignees
Labels

Comments

@alex-page
Copy link
Member

alex-page commented Apr 25, 2023

The focus outline is showing up incorrectly across product pages. We should address this and ensure that incorrect usage is seen as a failure in polaris-coverage.

  • Fix Admin issues in screenshots below
    • Fix changing focus outlines on RTE
    • Fix "Select products" focus outline showing on open
    • Fix "Sales channel" popup activator focus ring showing on open
  • Move https://github.com/Shopify/web/pull/83179 into polaris-stylelint to encourage teams to fix failures
  • Reduce the focus ring mixin to render only two lines of css
  • Share wins in "Quality wins" slack channel

image

image

Screen.Recording.2023-04-25.at.9.33.46.AM.mov
@alex-page alex-page changed the title Focus outline should only show on focus:visible Focus outline clean up and only show on focus:visible Apr 25, 2023
@sophschneider sophschneider self-assigned this Apr 25, 2023
@sophschneider
Copy link
Contributor

The first screenshot of the ProductResourcePicker is focusing on open because its an overridden Polaris ListBox, which usually has its first item focused with a blue background but is being overriden to be a focus ring instead. So its not a focus-visible issue and I think the best fix would be to refactor it to be an ActionList.

cc: @ouellettejordan @alex-page

@alex-page
Copy link
Member Author

alex-page commented Apr 28, 2023

@sophschneider feel free to get draft PR's up and make assumptions. It is going to be much easier to tophat a change then talk through it here. I trust you if you want to refactor that UI.

@sam-b-rose
Copy link
Member

sam-b-rose commented Aug 7, 2023

@sophschneider is this currently still in progress? Do you thing we should decide to either close it out and revisit later or move to the backlog?

@sophschneider
Copy link
Contributor

@samrose3 hmm yeah I think we should revisit it late/ move to the backlog. I think it would be cool if we did something similar to to aarons shadow bevel component for this

@sam-b-rose
Copy link
Member

Sounds good, thanks Sophie!

@sophschneider
Copy link
Contributor

I have a related prototype here for creating a focus ring mixin from a while ago #9467. The PR description has my prototype thoughts!

For the same reasoning in the PR description, I think the ShadowBevel pattern is the way to go

@sam-b-rose
Copy link
Member

This is great context!

I think the ShadowBevel pattern is the way to go

This wouldn't be a breaking change, correct? Just a feature add. Seems like we could address this after v12?

Copy link
Contributor

Hi! We noticed there hasn’t been activity on this issue in a while. After 30 days, it will close automatically.

If it’s still relevant, or you have updates, comment and let us know. And don’t worry, you can always re-open later if needed.

Copy link
Contributor

Hi! We noticed there hasn’t been activity on this issue in a while. After 30 days, it will close automatically.

If it’s still relevant, or you have updates, comment and let us know. And don’t worry, you can always re-open later if needed.

@alex-page
Copy link
Member Author

😭

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

No branches or pull requests

4 participants
@sam-b-rose @alex-page @sophschneider and others