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

fix(ProgressIndicator): fix focus style outline (#11773) #12213

Merged
merged 6 commits into from
Oct 6, 2022

Conversation

hannelevaltanen
Copy link
Contributor

Closes #11773

This fixes the ProgressIndicator focus style to follow the style guide.

Default
Screenshot 2022-09-11 at 11 53 59

Focus when using keyboard
Screenshot 2022-09-11 at 11 52 52

Hover
Screenshot 2022-09-11 at 11 52 22

Clicked
Screenshot 2022-09-11 at 11 53 38

Changelog

Changed

  • Changed the progress step button to focus the label only

Removed

  • Removed unnecessary focus styles

Testing / Reviewing

@hannelevaltanen hannelevaltanen requested a review from a team as a code owner October 2, 2022 10:35
@github-actions
Copy link
Contributor

github-actions bot commented Oct 2, 2022

DCO Assistant Lite bot All contributors have signed the DCO.

@netlify
Copy link

netlify bot commented Oct 2, 2022

Deploy Preview for carbon-components-react ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 156b1cb
🔍 Latest deploy log https://app.netlify.com/sites/carbon-components-react/deploys/633ee9e32099370008a67014
😎 Deploy Preview https://deploy-preview-12213--carbon-components-react.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Oct 2, 2022

Deploy Preview for carbon-elements ready!

Name Link
🔨 Latest commit 156b1cb
🔍 Latest deploy log https://app.netlify.com/sites/carbon-elements/deploys/633ee9e3fa75c60009a54e97
😎 Deploy Preview https://deploy-preview-12213--carbon-elements.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@hannelevaltanen
Copy link
Contributor Author

I have read the DCO document and I hereby sign the DCO.

Copy link
Collaborator

@tw15egan tw15egan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great. Thanks for contributing! 🎉

@tw15egan tw15egan requested review from a team and thyhmdo and removed request for a team October 3, 2022 17:38
Copy link
Member

@tay1orjones tay1orjones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fantastic, thank you!

@tw15egan
Copy link
Collaborator

tw15egan commented Oct 5, 2022

cc @thyhmdo does this focus state look correct?

@thyhmdo
Copy link
Member

thyhmdo commented Oct 5, 2022

Everything looks correct, except the Clicked state.
Text and underline (or text decoration?) change to Black on the active state when the step is a link, meaning that you can go back and forth.
We already have this in our storybook so just want to confirm this.

@thyhmdo
Copy link
Member

thyhmdo commented Oct 5, 2022

Here's the spec from Lauren. Active or when selected, the underline should also be 1px instead

image

@hannelevaltanen
Copy link
Contributor Author

hannelevaltanen commented Oct 6, 2022

Thank you all and thank you @thyhmdo for the spec! I can fix the :active state, so only 1px underline and $text-primary (#161616) token for the colors.

Screenshot 2022-10-06 at 7 41 58

@sstrubberg sstrubberg enabled auto-merge (squash) October 6, 2022 14:57
@sstrubberg sstrubberg merged commit 2cb3054 into carbon-design-system:main Oct 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[Bug]: Progress indicator focus style outline wrong
5 participants