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

Set Style sidebar panel initialOpen to false #24073

Closed
wants to merge 1 commit into from

Conversation

richtabor
Copy link
Member

Description

This PR sets the initialOpen prop to false for the Styles PanelBody which will clean up the Settings Sidebar UI and lowering cognitive overload. Closes #24072.

Screenshots

Before:

Screen Shot 2020-07-20 at 2 52 16 PM

After:

Screen Shot 2020-07-20 at 2 43 45 PM

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@richtabor richtabor self-assigned this Jul 20, 2020
@youknowriad
Copy link
Contributor

This has been explicitly changed to true in a previous PR #20617 cc @pento @jasmussen

@github-actions
Copy link

Size Change: 0 B

Total Size: 1.15 MB

ℹ️ 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 944 B 0 B
build/block-directory/style.css 945 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.61 kB 0 B
build/block-library/editor.css 7.61 kB 0 B
build/block-library/index.min.js 132 kB 0 B
build/block-library/style-rtl.css 7.82 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.6 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.8 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/editor/style-rtl.css 3.78 kB 0 B
build/editor/style.css 3.78 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.32 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

@richtabor
Copy link
Member Author

This has been explicitly changed to true in a previous PR #20617 cc @pento @jasmussen

Ah, I see. I still stand by the reason behind the change back. I'd appreciate feedback from @pento and @jasmussen for sure :)

@ZebulanStanphill ZebulanStanphill added the Needs Design Feedback Needs general design feedback. label Jul 21, 2020
@ZebulanStanphill
Copy link
Member

ZebulanStanphill commented Jul 21, 2020

It's kinda weird that Styles is even in the inspector in the first place. Every other control I can think of only appears in either the toolbar or the inspector. But Styles appears in both.

If I recall correctly (and correct me if I'm wrong), Styles was added to the inspector to make it more discoverable. But the inspector is supposed to be for secondary controls. It's where controls are supposed to be intentionally hidden to a degree.

To me at least, that seems to indicate that the real problem is that the purpose of the block/style switcher in the toolbar is not as obvious as it should be. The accessibility team has already pointed out some of the issues with the current switcher design.

If the block/style switcher's purpose was more obvious, we wouldn't need the Styles panel in the inspector at all.

@ZebulanStanphill
Copy link
Member

As for a more immediate action, though, I'm fine with this PR.

@pento
Copy link
Member

pento commented Jul 21, 2020

I agree that there appears to be too much going on in the sidebar in the screenshots of the Image block, but I don't think changing the initialOpen prop of the Styles panel is the correct solution here.

Take the Quote block, for example:

Starting with the Styles panel closed makes the sidebar appear underutilised, there's a huge amount of blank space not being used.

Or looking at Jetpack's Map block:

Closing the Styles panel improves things a little, but there's still a lot going on. I suspect the overload stems from having multiple panels open, one after the other. I'm wondering if a better approach might be to have a clearer boundary between panels. Eg,

(Disclaimer: this particular style obviously doesn't fit as a standalone change, but I think it illustrates the idea.)

@richtabor
Copy link
Member Author

Perhaps an optimal solution would be that the Styles panel remembers its open/closed state editor-wide (like how the Document panels do)? So if a use closes the Styles panel on one block, its closed throughout the editor. Is that possible?

@richtabor
Copy link
Member Author

I don't think its a matter of how much white-space is available, but rather how much cognitive load is present one way or the other. With a block such as the Quote block its not a big deal having it open - as there are no other controls - but for other blocks, even the JetPack Map block, its quickly an intensive experience.

Also, if JetPack - or even another third party plugin - adds more map styles (which isn't out of the question), the styles will keep pushing the others controls out of the way, and keep adding to the cognitive load.

@jasmussen
Copy link
Contributor

I don't think its a matter of how much white-space is available, but rather how much cognitive load is present one way or the other.

I think this is a good observation. However I disagree with changing it to be collapsed by default.

Part of the purpose of the G2 design revamp was exactly that, to reduce, simplify, optimize, combine and lighten this very cognitive load, editor-wide.

We did not get to the sidebar with those efforts. But that doesn't mean we shouldn't. There are many many efforts that can be taken to simplify things here, including finding a better grid system for controls inside, reducing borders where possible, and making labels consistent and in the same spot, add contextuality where possible. Many of my recent PRs have taken steps towards that, including by reducing the shades of gray, and by emerging steps for a grid system. I hope to continue that work on a per-control basis, work with @ItsJonQ to improve things there.

This is a much harder process than simply collapsing by default. But ultimately, I believe it is the better approach. By allowing things to collapse we are, to an extent, encouraging kitchen sink sidebar design. Whereas if we attack head on what makes it kitchen-sinky, we can benefit the issue at the core.

In this case, I do agree that the style picker is in a bad state. For starters we've done a little work to improve it directly in the switcher:

Screenshot 2020-08-03 at 10 37 29

I'd love if we found a way to make this feel natural enough that it might even be in the switcher only, but I'm not confident in that.

In the mean time, what are the biggest problem with the style picker display in the sidebar? To me:

  • "Default style" feels like it's using a temp design
  • The thumbnails could be visually simplified a bit, there are some mockups from Pablo I've seen for patterns, that might reduce this a bit.
  • At the very least, the spacing between thumbnails and all around the panel, as well as the minimum default height could be massaged to make the entire panel be at least a third smaller, if not more.

All that would help.

Base automatically changed from master to trunk March 1, 2021 15:43
@skorasaurus
Copy link
Member

@richtabor

The style sidebar panel has been redesigned (no longer including the image preview of style in the sidepanel) quite a bit and the initial issue has been closed (#24072); want me to close this ?

@richtabor
Copy link
Member Author

richtabor commented Feb 22, 2022

The current implementation is much better. Closing.

@richtabor richtabor closed this Feb 22, 2022
@richtabor richtabor deleted the try/24072/close-styles-panel branch February 22, 2022 01:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Design Feedback Needs general design feedback.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Initially close the Block Styles sidebar panel
6 participants