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

dont display outline when interacting with links #1293

Closed
wants to merge 2 commits into from

Conversation

moimikey
Copy link
Contributor

Issue:

What I did

This simply adds a line of css to react (and not native since it's not through a browser) to get rid of the visually disturbing outline that shows when you click on hyperlinks.

How to test

You'll simply not see the box shadow anymore.

screen shot 2017-06-15 at 4 50 09 pm

Where previously you could:

screen shot 2017-06-15 at 5 01 29 pm

@codecov
Copy link

codecov bot commented Jun 15, 2017

Codecov Report

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

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #1293   +/-   ##
======================================
  Coverage    14.1%   14.1%           
======================================
  Files         201     201           
  Lines        4609    4609           
  Branches      494     502    +8     
======================================
  Hits          650     650           
+ Misses       3532    3523    -9     
- Partials      427     436    +9
Impacted Files Coverage Δ
app/react/src/server/index.html.js 0% <ø> (ø) ⬆️
...rc/modules/ui/components/left_panel/text_filter.js 33.33% <0%> (ø) ⬆️
addons/knobs/src/components/types/Object.js 5.81% <0%> (ø) ⬆️
addons/knobs/src/components/WrapStory.js 12% <0%> (ø) ⬆️
lib/ui/src/modules/ui/configs/handle_routing.js 28.94% <0%> (ø) ⬆️
lib/ui/src/modules/ui/configs/handle_keyevents.js 33.33% <0%> (ø) ⬆️
lib/ui/src/modules/shortcuts/actions/shortcuts.js 6.25% <0%> (ø) ⬆️
addons/info/src/components/markdown/code.js 0% <0%> (ø) ⬆️
lib/ui/src/modules/ui/components/layout/usplit.js 38.7% <0%> (ø) ⬆️
lib/ui/src/modules/ui/containers/layout.js 12.5% <0%> (ø) ⬆️
... and 18 more

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 188876e...b75d55d. Read the comment docs.

Copy link
Member

@ndelangen ndelangen left a comment

Choose a reason for hiding this comment

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

Hey thanks for the PR!

I get it's ugly, but it's vital for keyboard usage.

Removing it without any other feedback is not really an option.

Can you find a solution so keyboard users do get feedback what they have focussed?

@shilman
Copy link
Member

shilman commented Jun 15, 2017

@moimikey thanks for the fix! i'm wondering whether the style should be scoped somehow? right now it's a global change and that feels a little dirty to me?

@danielduan
Copy link
Member

+1 on accessibility. it's nice to have it highlighted for kb usage.

@usulpro
Copy link
Member

usulpro commented Jul 1, 2017

I wonder whether the use case of keyboard navigation includes stories switching (if it's about Tab / Shift-Tab)? Asking because Ctrl-Shift-Arrow looks more comfortable for that. So maybe we can add special keyboard shortcuts for switching through the add-ons tabs as well? In this case, we can leave the outline only in the preview area?

@ndelangen
Copy link
Member

I'm interested in a quick-win fix for this that does not entail completely remove the focus styling.

In the long run I'll be moving our UI components and change the way styling works using glamorous.
And at that point I'll address this myself.

Please re-open if you can find a good style for focussed items 👍

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

Successfully merging this pull request may close these issues.

5 participants