Skip to content
This repository has been archived by the owner on Dec 8, 2022. It is now read-only.

Updated focus style to be more subtle #8

Merged
merged 2 commits into from
Nov 7, 2018

Conversation

blackbaud-conorwright
Copy link
Contributor

@blackbaud-conorwright blackbaud-conorwright commented Oct 26, 2018

Resolves: blackbaud/skyux2#2146
Docs: No changes required

@codecov-io
Copy link

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master       #8   +/-   ##
=======================================
  Coverage   99.54%   99.54%           
=======================================
  Files          21       21           
  Lines         437      437           
  Branches       66       66           
=======================================
  Hits          435      435           
  Misses          2        2

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 922f965...20d5cdf. Read the comment docs.

@@ -41,6 +41,10 @@
.sky-modal-content {
background-color: #fff;
z-index: 1;

&:focus {
outline: thin dotted #4d90fe;

Choose a reason for hiding this comment

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

Is this @Blackbaud-ToddRoberts approved?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's how I recreated what he suggested ;p
But I haven't received feedback from him on it yet (and he's out until Monday)

Choose a reason for hiding this comment

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

I think it looks generally ok; only slight concern is that if this is across browsers, it will look more prominent than other focus in Firefox where the dotted line is black, but if it has to be the same it's not a showstopper

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to use the browser's color of choice

Copy link

@Blackbaud-ToddRoberts Blackbaud-ToddRoberts left a comment

Choose a reason for hiding this comment

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

This approach looks good to me.

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

Successfully merging this pull request may close these issues.

4 participants