Skip to content
This repository has been archived by the owner on Mar 4, 2020. It is now read-only.

fix(button): removed dotted outline in firefox #390

Merged
merged 7 commits into from
Oct 30, 2018

Conversation

bmdalex
Copy link
Collaborator

@bmdalex bmdalex commented Oct 23, 2018

Button

This PR removes the dotted outline in Firefox for Button component. Repro:

  • go to doc site in Firefox;
  • navigate to the buttons using keyboard navigation;
  • observe focus outline.

Before

screen shot 2018-10-22 at 23 29 13

After

screen shot 2018-10-23 at 11 55 18

Copy link
Collaborator Author

@bmdalex bmdalex left a comment

Choose a reason for hiding this comment

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

@levithomason
@kuzhelov
pls take a look at my comments

@bmdalex bmdalex added 🚀 ready for review question Further information is requested, concerns that require additional thought are raised labels Oct 23, 2018
@codecov
Copy link

codecov bot commented Oct 23, 2018

Codecov Report

Merging #390 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #390   +/-   ##
=======================================
  Coverage   91.64%   91.64%           
=======================================
  Files          41       41           
  Lines        1341     1341           
  Branches      197      172   -25     
=======================================
  Hits         1229     1229           
  Misses        108      108           
  Partials        4        4

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2e8964f...7b7a541. Read the comment docs.

@bmdalex bmdalex force-pushed the fix/button-fix-outline-firefox branch from 83c22b8 to 5f7e5cb Compare October 23, 2018 10:11
@bmdalex bmdalex force-pushed the fix/button-fix-outline-firefox branch from 5f7e5cb to 18d50bd Compare October 23, 2018 14:12
@bmdalex bmdalex added 🚀 ready for review and removed question Further information is requested, concerns that require additional thought are raised ready for merge labels Oct 23, 2018
Copy link
Contributor

@notandrew notandrew left a comment

Choose a reason for hiding this comment

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

I don't think the changes should be made within normalize -- if we update to a new version of normalize, this change will be lost. Should it go into CustomCSS or siteVariables?

@kuzhelov
Copy link
Contributor

kuzhelov commented Oct 23, 2018

great point, thanks @notandrew - that's definitely would be a better approach 👍

@kuzhelov kuzhelov added needs author feedback Author's opinion is asked and removed 🚀 ready for review labels Oct 23, 2018
@bmdalex bmdalex added 🚀 ready for review and removed needs author feedback Author's opinion is asked labels Oct 24, 2018
@bmdalex
Copy link
Collaborator Author

bmdalex commented Oct 24, 2018

@kuzhelov
addressed @notandrew 's comments and moved the change in the semanticCssOverrides overrides file as per our discussion; ready to merge?

[type="reset"]:-moz-focusring,
[type="submit"]:-moz-focusring {
outline: 0;
}
Copy link
Member

Choose a reason for hiding this comment

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

This will only work in the doc site, is that the intent? If this should be the style for all usages of the Teams theme, including outside of the doc site by consumers, then this should be a staticStyle in the Teams theme. This way, everywhere the Teams theme is used, there is no outline in Firefox.

Copy link
Contributor

Choose a reason for hiding this comment

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

for now yes, the intent was just to address doc site issue and propose a question of whether the same change should be applied for Teams theme.

@kuzhelov kuzhelov merged commit 2e2d97f into master Oct 30, 2018
@layershifter layershifter deleted the fix/button-fix-outline-firefox branch November 9, 2018 09:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants