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

Use a pseudo element to prevent inspector tab width from changing when selected #9793

Merged
merged 5 commits into from Sep 13, 2018

Conversation

Projects
None yet
3 participants
@chrisvanpatten
Contributor

chrisvanpatten commented Sep 11, 2018

This is a very small tweak, but it's one of those little things that makes me crazy ;-)

It adds a pseudo element to the inspector tabs, with the content set to match the tab label. The psuedo-element is bolded, so the width of the tab is always the same. It's sort of hard to explain in text so here's an example:

Before:
tab-normal

After:
tab-fixed

And to help make it a bit clearer what's happening under-the-hood, here's how it looks if the pseudo element is visible (it's the 2nd line of labels):

tab-example

That second label is pre-bolded, so when the focus changes and the visible label changes to bold, the width is unchanged.

There's probably a better way to have the data attribute on the JS side (on the Block tab in particular, since there's some logic around selected block count that I just duplicated) but I'm not sure what's most appropriate there.

@chrisvanpatten

This comment has been minimized.

Show comment
Hide comment
@chrisvanpatten

chrisvanpatten Sep 11, 2018

Contributor

It might also be good to get an accessibility review on this; I think the pseudo element will be invisible to screen readers (which is desired!) since there's an aria-label, but I don't have access to a screen reader environment at the moment unfortunately.

Contributor

chrisvanpatten commented Sep 11, 2018

It might also be good to get an accessibility review on this; I think the pseudo element will be invisible to screen readers (which is desired!) since there's an aria-label, but I don't have access to a screen reader environment at the moment unfortunately.

@jasmussen

This comment has been minimized.

Show comment
Hide comment
@jasmussen

jasmussen Sep 12, 2018

Contributor

This is DAMN clever.

I didn't even notice the little jump until now, and now I want it gone. I love this, from a design point of view, 👍 👍.

Have you tested in IE11? If it doesn't work there you can use @supports (position:sticky) {} to wrap the rule. I don't expect any accessibility challenges, and if there are could we use speak: none;? Adding a label to be sure.

Contributor

jasmussen commented Sep 12, 2018

This is DAMN clever.

I didn't even notice the little jump until now, and now I want it gone. I love this, from a design point of view, 👍 👍.

Have you tested in IE11? If it doesn't work there you can use @supports (position:sticky) {} to wrap the rule. I don't expect any accessibility challenges, and if there are could we use speak: none;? Adding a label to be sure.

@chrisvanpatten

This comment has been minimized.

Show comment
Hide comment
@chrisvanpatten

chrisvanpatten Sep 12, 2018

Contributor

Per this document CSS content is sometimes read by screen readers, so I went ahead and added speak: none;.

Will give a more thorough test to IE11 later for safety, but the technique definitely works; we use it in production on one of our recent client projects which is IE11 compatible.

Contributor

chrisvanpatten commented Sep 12, 2018

Per this document CSS content is sometimes read by screen readers, so I went ahead and added speak: none;.

Will give a more thorough test to IE11 later for safety, but the technique definitely works; we use it in production on one of our recent client projects which is IE11 compatible.

@jasmussen jasmussen requested a review from WordPress/gutenberg-core Sep 13, 2018

@tofumatt

I love it. I'm gonna test it in IE 11 now and make a few tweaks as per my comments. But the concept is good so I'll push some changes and then approve 😄

Show outdated Hide outdated edit-post/components/sidebar/settings-header/index.js
@@ -17,6 +17,18 @@
color: $dark-gray-900;
@include square-style__neutral;
// This pseudo-element "duplicates" the tab label and sets the text to bold.
// This ensures that the tab doesn't change width when selected.

This comment has been minimized.

@tofumatt

tofumatt Sep 13, 2018

Member

Good description; I'll add a link to this PR (just in case git blame eventually doesn't lead someone back to here) for context.

@tofumatt

tofumatt Sep 13, 2018

Member

Good description; I'll add a link to this PR (just in case git blame eventually doesn't lead someone back to here) for context.

tofumatt added some commits Sep 13, 2018

@tofumatt tofumatt added this to the 3.9 milestone Sep 13, 2018

@tofumatt

Tested in IE 11 and I dig it! Thanks for fixing this; it bugged me as well 😄

@chrisvanpatten chrisvanpatten merged commit e81e50c into WordPress:master Sep 13, 2018

2 checks passed

codecov/project No report found to compare against
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@chrisvanpatten

This comment has been minimized.

Show comment
Hide comment
@chrisvanpatten

chrisvanpatten Sep 13, 2018

Contributor

Thanks @tofumatt! 🚢

Contributor

chrisvanpatten commented Sep 13, 2018

Thanks @tofumatt! 🚢

@chrisvanpatten chrisvanpatten deleted the TomodomoCo:tweak/inspector-tab-width branch Sep 13, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment