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

Avoid focus style from being cut on the categories panel #24197

Merged
merged 2 commits into from
Jul 27, 2020

Conversation

youknowriad
Copy link
Contributor

closes #23989

That overflow was not necessary.

Testing instructions

  • Add some categories to the categories panel
  • Focus the checkboxes
  • The focus style appears properly.

@youknowriad youknowriad added [Type] Bug An existing feature does not function as intended [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta labels Jul 24, 2020
@youknowriad youknowriad self-assigned this Jul 24, 2020
@github-actions
Copy link

github-actions bot commented Jul 24, 2020

Size Change: +33 B (0%)

Total Size: 1.15 MB

Filename Size Change
build/editor/style-rtl.css 3.8 kB +17 B (0%)
build/editor/style.css 3.79 kB +16 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.min.js 1.14 kB 0 B
build/annotations/index.min.js 3.67 kB 0 B
build/api-fetch/index.min.js 3.43 kB 0 B
build/autop/index.min.js 2.82 kB 0 B
build/blob/index.min.js 620 B 0 B
build/block-directory/index.min.js 7.63 kB 0 B
build/block-directory/style-rtl.css 953 B 0 B
build/block-directory/style.css 952 B 0 B
build/block-editor/index.min.js 125 kB 0 B
build/block-editor/style-rtl.css 10.8 kB 0 B
build/block-editor/style.css 10.8 kB 0 B
build/block-library/editor-rtl.css 7.63 kB 0 B
build/block-library/editor.css 7.63 kB 0 B
build/block-library/index.min.js 132 kB 0 B
build/block-library/style-rtl.css 7.83 kB 0 B
build/block-library/style.css 7.83 kB 0 B
build/block-library/theme-rtl.css 728 B 0 B
build/block-library/theme.css 729 B 0 B
build/block-serialization-default-parser/index.min.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.min.js 3.1 kB 0 B
build/blocks/index.min.js 48.3 kB 0 B
build/components/index.min.js 200 kB 0 B
build/components/style-rtl.css 15.7 kB 0 B
build/components/style.css 15.6 kB 0 B
build/compose/index.min.js 9.68 kB 0 B
build/core-data/index.min.js 11.5 kB 0 B
build/data-controls/index.min.js 1.29 kB 0 B
build/data/index.min.js 8.45 kB 0 B
build/date/index.min.js 5.38 kB 0 B
build/deprecated/index.min.js 772 B 0 B
build/dom-ready/index.min.js 568 B 0 B
build/dom/index.min.js 3.23 kB 0 B
build/edit-navigation/index.min.js 10.8 kB 0 B
build/edit-navigation/style-rtl.css 1.08 kB 0 B
build/edit-navigation/style.css 1.08 kB 0 B
build/edit-post/index.min.js 304 kB 0 B
build/edit-post/style-rtl.css 5.61 kB 0 B
build/edit-post/style.css 5.61 kB 0 B
build/edit-site/index.min.js 16.9 kB 0 B
build/edit-site/style-rtl.css 3.06 kB 0 B
build/edit-site/style.css 3.06 kB 0 B
build/edit-widgets/index.min.js 9.37 kB 0 B
build/edit-widgets/style-rtl.css 2.45 kB 0 B
build/edit-widgets/style.css 2.45 kB 0 B
build/editor/editor-styles-rtl.css 537 B 0 B
build/editor/editor-styles.css 539 B 0 B
build/editor/index.min.js 45.3 kB 0 B
build/element/index.min.js 4.65 kB 0 B
build/escape-html/index.min.js 733 B 0 B
build/format-library/index.min.js 7.72 kB 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.min.js 2.13 kB 0 B
build/html-entities/index.min.js 621 B 0 B
build/i18n/index.min.js 3.56 kB 0 B
build/is-shallow-equal/index.min.js 711 B 0 B
build/keyboard-shortcuts/index.min.js 2.51 kB 0 B
build/keycodes/index.min.js 1.94 kB 0 B
build/list-reusable-blocks/index.min.js 3.12 kB 0 B
build/list-reusable-blocks/style-rtl.css 476 B 0 B
build/list-reusable-blocks/style.css 476 B 0 B
build/media-utils/index.min.js 5.33 kB 0 B
build/notices/index.min.js 1.79 kB 0 B
build/nux/index.min.js 3.4 kB 0 B
build/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B
build/plugins/index.min.js 2.56 kB 0 B
build/primitives/index.min.js 1.4 kB 0 B
build/priority-queue/index.min.js 789 B 0 B
build/redux-routine/index.min.js 2.85 kB 0 B
build/rich-text/index.min.js 13.9 kB 0 B
build/server-side-render/index.min.js 2.71 kB 0 B
build/shortcode/index.min.js 1.7 kB 0 B
build/token-list/index.min.js 1.27 kB 0 B
build/url/index.min.js 4.06 kB 0 B
build/viewport/index.min.js 1.85 kB 0 B
build/warning/index.min.js 1.14 kB 0 B
build/wordcount/index.min.js 1.17 kB 0 B

compressed-size-action

@@ -1,6 +1,5 @@
.editor-post-taxonomies__hierarchical-terms-list {
max-height: 14em;
overflow: auto;
Copy link
Member

@ocean90 ocean90 Jul 26, 2020

Choose a reason for hiding this comment

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

We can't remove that because the list has a limited height, it needs to be scrollable.

I was able to fix it by using negative margin with the width of the focus outline.

margin-left: -5.5px;
padding-left: 5.5px;
margin-top: -5.5px;
padding-top: 5.5px;

This would replace the padding-left: 2px; workaround in line 5/6. 5.5 pixels is the result of $border-width * 4 + $border-width-focus as used here.

Though, I noticed in Chrome that it has some rendering issues with the half pixel value:

image

The top/left focus outline of the first input field is smaller than the other fields.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, how did I miss that :)

I applied the suggestion and I think the half-pixel issue is not unique to this particular case, it's something @MichaelArestad is trying to fix separately across the UI.

Copy link
Contributor

@MichaelArestad MichaelArestad Jul 27, 2020

Choose a reason for hiding this comment

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

@youknowriad I have tried two methods. Neither works right now. I ended up having to shelve it for now until either PostCSS updates to work properly (was failing when encountering calc() within a box shadow) or someone comes up with another idea. :/ It was just taking too much time. I will definitely revisit as the random border widths on 1x devices make the whole editor look bad.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe we should just abandon the 1.5 borders.

Copy link
Contributor

Choose a reason for hiding this comment

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

@youknowriad That's certainly the simplest solution.

@youknowriad youknowriad force-pushed the fix/categories-panel branch 2 times, most recently from 6c98692 to 66a027f Compare July 27, 2020 07:33
@youknowriad youknowriad force-pushed the fix/categories-panel branch from 66a027f to 03edac1 Compare July 27, 2020 07:34
@youknowriad youknowriad dismissed ocean90’s stale review July 27, 2020 07:37

Should be fine now

@youknowriad youknowriad merged commit 0f1f44d into master Jul 27, 2020
@youknowriad youknowriad deleted the fix/categories-panel branch July 27, 2020 08:10
@github-actions github-actions bot added this to the Gutenberg 8.7 milestone Jul 27, 2020
@youknowriad youknowriad removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Jul 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Accessibility regression: The Categories checkboxes focus style is cut-off
3 participants