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

fix(button): update focus styles to use box-shadow #4867

Merged
merged 1 commit into from
Dec 13, 2019
Merged

fix(button): update focus styles to use box-shadow #4867

merged 1 commit into from
Dec 13, 2019

Conversation

joshblack
Copy link
Contributor

Closes #3919
Closes #3560

Changelog

New

Changed

  • Update button focus styles to use box-shadow instead of outline as the offset for outline is unsupported in IE11

Removed

Testing / Reviewing

  • Run environment in IE11 and verify focus styles are applied

@joshblack joshblack requested a review from a team as a code owner December 12, 2019 17:24
@ghost ghost requested review from aledavila and emyarod December 12, 2019 17:24
@netlify
Copy link

netlify bot commented Dec 12, 2019

Deploy preview for the-carbon-components ready!

Built with commit 21ddef7

https://deploy-preview-4867--the-carbon-components.netlify.com

@netlify
Copy link

netlify bot commented Dec 12, 2019

Deploy preview for carbon-elements ready!

Built with commit 21ddef7

https://deploy-preview-4867--carbon-elements.netlify.com

@netlify
Copy link

netlify bot commented Dec 12, 2019 •
edited
Loading

Deploy preview for carbon-components-react failed.

Built with commit 21ddef7

https://app.netlify.com/sites/carbon-components-react/deploys/5df2c3315d3817ecc5cae791

@joshblack
Copy link
Contributor Author

Before anyone merges in, just wanted to confirm that we all checked in different browsers and what-not? Should we check other components that use button as well? @aledavila @abbeyhrt 👀

I'll make sure to do another pass too just to confirm 👍

@joshblack joshblack requested a review from a team December 12, 2019 22:20
@ghost ghost requested a review from jeanservaas December 12, 2019 22:20
@ghost ghost removed their request for review December 12, 2019 22:20
@abbeyhrt
Copy link
Contributor

@joshblack I check Safari, Chrome, and FF on Mac and it all looked good!

@abbeyhrt abbeyhrt closed this Dec 12, 2019
@abbeyhrt abbeyhrt reopened this Dec 12, 2019
@abbeyhrt
Copy link
Contributor

Ignore the close! That was on accident haha

jeanservaas
jeanservaas approved these changes Dec 13, 2019
View reviewed changes
Copy link
Contributor

@jeanservaas jeanservaas left a comment

Choose a reason for hiding this comment

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

lgtm!

@aledavila
Copy link
Contributor

Yes i checked on all browsers in windows .. not mac

@joshblack joshblack merged commit e626cdf into carbon-design-system:master Dec 13, 2019
@joshblack joshblack deleted the 3919-update-button-focus-styles branch December 13, 2019 19:47
joshblack added a commit to joshblack/carbon that referenced this pull request Jan 6, 2020
@joshblack
fix(button): update focus styles to use box-shadow (carbon-design-sys…
8495b36 
…tem#4867)
@joshblack joshblack mentioned this pull request Jan 6, 2020
Closed
@emyarod emyarod mentioned this pull request Jan 30, 2020
Merged
@joshblack joshblack mentioned this pull request Jan 31, 2020
Closed
@joshblack joshblack mentioned this pull request Feb 25, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeanservaas jeanservaas jeanservaas approved these changes

@abbeyhrt abbeyhrt abbeyhrt approved these changes

@aledavila aledavila aledavila approved these changes

@emyarod emyarod Awaiting requested review from emyarod

Assignees
No one assigned
Labels
status: visual review 🔍
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Buttons don't have tab focus in WHCM Button focus border issues
4 participants
@joshblack @abbeyhrt @aledavila @jeanservaas