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

Polish sidebar controls #23578

Merged
merged 7 commits into from
Jul 2, 2020
Merged

Polish sidebar controls #23578

merged 7 commits into from
Jul 2, 2020

Conversation

jasmussen
Copy link
Contributor

This PR polishes controls in the sidebar.

  • It enhances the base control to have a minimum height that fits a button size.
  • It changes the status & visibility buttons to be more harmoniously placed.
  • It changes the "Move to Trash" button to use sentence case, and be a tertiary button.

Screenshot 2020-06-30 at 10 29 39

It styles the checkbox control to better match the styles outlined in #18667:

Screenshot 2020-06-30 at 11 09 29

It styles the toggle control the same way:

Screenshot 2020-06-30 at 11 13 52

... And the radio control:

Screenshot 2020-06-30 at 11 25 01

It fixes a bug where the post-publish sidebar was 1px off. Before:

Screenshot 2020-06-30 at 11 24 47

After:

Screenshot 2020-06-30 at 11 26 13

@jasmussen jasmussen added the [Type] Enhancement A suggestion for improvement. label Jun 30, 2020
@jasmussen jasmussen self-assigned this Jun 30, 2020
@github-actions
Copy link

github-actions bot commented Jun 30, 2020

Size Change: -135 B (0%)

Total Size: 1.13 MB

Filename Size Change
build/block-directory/index.js 7.42 kB +30 B (0%)
build/block-editor/index.js 109 kB -100 B (0%)
build/block-editor/style-rtl.css 10.7 kB +1 B
build/block-library/editor-rtl.css 7.63 kB +145 B (1%)
build/block-library/editor.css 7.63 kB +144 B (1%)
build/block-library/index.js 129 kB +209 B (0%)
build/block-library/style-rtl.css 7.79 kB -249 B (3%)
build/block-library/style.css 7.79 kB -255 B (3%)
build/block-serialization-default-parser/index.js 1.88 kB +1 B
build/components/index.js 198 kB +1 B
build/components/style-rtl.css 15.9 kB -47 B (0%)
build/components/style.css 15.8 kB -48 B (0%)
build/core-data/index.js 11.4 kB +1 B
build/data/index.js 8.44 kB +2 B (0%)
build/date/index.js 5.47 kB +3 B (0%)
build/edit-navigation/index.js 9.88 kB +11 B (0%)
build/edit-post/index.js 304 kB +72 B (0%)
build/edit-post/style-rtl.css 5.54 kB +35 B (0%)
build/edit-post/style.css 5.54 kB +33 B (0%)
build/edit-site/index.js 16.6 kB -3 B (0%)
build/edit-site/style-rtl.css 3 kB -34 B (1%)
build/edit-site/style.css 3 kB -36 B (1%)
build/editor/index.js 44.9 kB +18 B (0%)
build/editor/style-rtl.css 3.82 kB -36 B (0%)
build/editor/style.css 3.82 kB -40 B (1%)
build/element/index.js 4.65 kB +2 B (0%)
build/format-library/index.js 7.72 kB -1 B
build/i18n/index.js 3.56 kB -1 B
build/is-shallow-equal/index.js 710 B -1 B
build/keyboard-shortcuts/index.js 2.52 kB +6 B (0%)
build/redux-routine/index.js 2.85 kB +2 B (0%)
build/shortcode/index.js 1.69 kB -1 B
build/wordcount/index.js 1.17 kB +1 B
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.62 kB 0 B
build/api-fetch/index.js 3.4 kB 0 B
build/autop/index.js 2.82 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/style-rtl.css 941 B 0 B
build/block-directory/style.css 942 B 0 B
build/block-editor/style.css 10.7 kB 0 B
build/block-library/theme-rtl.css 730 B 0 B
build/block-library/theme.css 732 B 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 48.2 kB 0 B
build/compose/index.js 9.65 kB 0 B
build/data-controls/index.js 1.29 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 569 B 0 B
build/dom/index.js 3.19 kB 0 B
build/edit-navigation/style-rtl.css 1.02 kB 0 B
build/edit-navigation/style.css 1.02 kB 0 B
build/edit-widgets/index.js 9.32 kB 0 B
build/edit-widgets/style-rtl.css 2.42 kB 0 B
build/edit-widgets/style.css 2.42 kB 0 B
build/editor/editor-styles-rtl.css 537 B 0 B
build/editor/editor-styles.css 539 B 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.js 2.13 kB 0 B
build/html-entities/index.js 622 B 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.12 kB 0 B
build/list-reusable-blocks/style-rtl.css 450 B 0 B
build/list-reusable-blocks/style.css 451 B 0 B
build/media-utils/index.js 5.29 kB 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/index.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.js 2.56 kB 0 B
build/primitives/index.js 1.5 kB 0 B
build/priority-queue/index.js 788 B 0 B
build/rich-text/index.js 14 kB 0 B
build/server-side-render/index.js 2.67 kB 0 B
build/token-list/index.js 1.28 kB 0 B
build/url/index.js 4.06 kB 0 B
build/viewport/index.js 1.85 kB 0 B
build/warning/index.js 1.14 kB 0 B

compressed-size-action

@jasmussen jasmussen requested a review from a team July 1, 2020 07:28
Copy link
Contributor

@mapk mapk left a comment

Choose a reason for hiding this comment

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

🎉 These little polishes go a long way! Thanks for catching them. It all looked and worked as you mentioned and feels really sharp.

@enriquesanchez
Copy link
Contributor

Amazing attention to detail! Tested it and it all worked as expected 👍

Copy link
Contributor

@tellthemachines tellthemachines left a comment

Choose a reason for hiding this comment

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

Great improvements! A couple of small things I noticed testing in Windows in high contrast mode:

  • The "move to trash" button now doesn't look like a button or link at all:

button-hi-c

I guess this is a problem for all tertiary buttons. Could we give them an outline of some kind that would be distinguishable from the focus outline? Or perhaps a transparent bottom border or something?

  • There's no way of telling which radio button is selected when they are not focused:
    (this was an existing issue, not sure what the best solution might be)

radio-hi-contrast

@jasmussen
Copy link
Contributor Author

Thank you Isabel, those are wonderful observations. I'll get right on those and we'll ship this after.

@jasmussen
Copy link
Contributor Author

I added a dotted transparent outline around tertiary buttons, which shows up in high contrast mode:

Screenshot 2020-07-02 at 11 25 00

Note that the above screenshot is scaled down so it gets fuzzy, but the outline is plenty visible.

I can't reproduce the radio button issue, to me they look correct regardless of focus:

Screenshot 2020-07-02 at 11 26 08

Where did you see this issue?

Thanks again.

@jasmussen
Copy link
Contributor Author

I'm going to merge this one with the reviews, and I will be happy to address any followups.

@jasmussen jasmussen merged commit 4fe793a into master Jul 2, 2020
@jasmussen jasmussen deleted the polish/sidebar-controls branch July 2, 2020 09:53
@github-actions github-actions bot added this to the Gutenberg 8.5 milestone Jul 2, 2020
@tellthemachines
Copy link
Contributor

Where did you see this issue?

On both Firefox and IE11, Windows 10, in high contrast mode. The only browser that seems to be showing them correctly is Edge (both the legacy and the new Chromium-based version)

@jasmussen
Copy link
Contributor Author

The only browser that seems to be showing them correctly is Edge (both the legacy and the new Chromium-based version)

Ah, yes, both Firefox and Chrome have historically not leveraged the high contrast mode very well. I believe Chrome didn't support it for the longest time virtually at all, though perhaps with chromium based edge that might have changed recently.

I'm going to take a look at a followup next week. Thanks for the clarification!

@jasmussen
Copy link
Contributor Author

#23706 fixes the checkbox control issue.

If you know of any existing tickets that closes, would appreciate if you add it as a "fixes" comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants