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

Move document settings into dialog. #25353

Closed
wants to merge 3 commits into from

Conversation

ZebulanStanphill
Copy link
Member

@ZebulanStanphill ZebulanStanphill commented Sep 15, 2020

Description

This PR moves the post/document settings out of the "general sidebar" and into a dedicated dialog modal, similar to the existing Options or Welcome Guide dialog modals.

The block inspector remains in the general sidebar, though I intend to make another PR in the future to try moving it into a dialog and moving the button to open it into the block toolbar. But that's likely to be a much more controversial move, so I'm not going to try and do it in this PR.

I also intend to move the pre-publish panel to a dialog modal in yet another PR, in case anyone is wondering. That's another part of the UI where great accessibility gains can be made by switching from a sidebar to a dialog modal.

I've also gone ahead and put the button to open the document settings before "Preview", "Save draft", and "Publish". I figure that the save/publish buttons should probably be the last thing in the toolbar. The block inspector button (formerly the "Settings" button) remains where it is to keep it next to the other plugin sidebar buttons.

The benefits of a dialog-ified document settings include:

  • Simpler and more accessible keyboard navigation and focus management.
  • When the user goes to edit the document settings, they are now the focus of the screen. Before, attention was divided between them and the rest of the editor.
  • The document settings and block inspector were never really related, so it was weird to have them share a tabbed sidebar. This PR makes the distinction between the two more obvious. Now, when you want to edit the post settings, you should know exactly what to do. No weird "click the title and the block inspector becomes the document settings" behavior.
  • Way more horizontal room on desktop screens for the document settings. Potentially, we could move all the "footer metaboxes" into this dialog, if that makes sense, and we'd have plenty of room.

How has this been tested?

I've made sure that opening/closing the document settings works as expected, and so do the interactions with the block inspector.

Screenshots

image
image
image

Checklist:

  • Apply the changes to the site editor and other screens (if necessary)
  • Figure out what to do regarding metaboxes in the "side" area.
  • 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.

@ZebulanStanphill ZebulanStanphill added [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Feature] Document Settings Document settings experience Needs Design Feedback Needs general design feedback. Needs Accessibility Feedback Need input from accessibility labels Sep 15, 2020
@ZebulanStanphill
Copy link
Member Author

There's one thing I haven't figured out regarding this big change: what do we do about the metaboxes in the "side" area? I haven't tested yet, but I assume trying to use the up/down buttons to move metaboxes from the footer area below the main WYSIWYG canvas to the metabox area in the document settings dialog won't work. To be fair, it's only very, very recently that moving them from one area to another even became possible in the block editor. (Drag-and-drop never worked and still doesn't.)

I figure that we should probably show all the metaboxes in one place anyway, but I'm not sure where. Should they all go in the footer area? Or should we put them all in the document settings dialog? There's certainly enough horizontal space to do the latter now.

@github-actions
Copy link

github-actions bot commented Sep 15, 2020

Size Change: -174 B (0%)

Total Size: 1.2 MB

Filename Size Change
build/block-directory/index.js 8.41 kB +1 B
build/block-editor/index.js 129 kB -3 B (0%)
build/block-library/index.js 134 kB -4 B (0%)
build/blocks/index.js 47.5 kB +2 B (0%)
build/components/index.js 202 kB -4 B (0%)
build/edit-post/index.js 306 kB +127 B (0%)
build/edit-post/style-rtl.css 6.09 kB -147 B (2%)
build/edit-post/style.css 6.08 kB -145 B (2%)
build/edit-site/index.js 19.5 kB +1 B
build/edit-widgets/index.js 17 kB -2 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.52 kB 0 B
build/api-fetch/index.js 3.33 kB 0 B
build/autop/index.js 2.72 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/style-rtl.css 943 B 0 B
build/block-directory/style.css 942 B 0 B
build/block-editor/style-rtl.css 11.1 kB 0 B
build/block-editor/style.css 11.1 kB 0 B
build/block-library/editor-rtl.css 8.59 kB 0 B
build/block-library/editor.css 8.59 kB 0 B
build/block-library/style-rtl.css 7.6 kB 0 B
build/block-library/style.css 7.59 kB 0 B
build/block-library/theme-rtl.css 741 B 0 B
build/block-library/theme.css 741 B 0 B
build/block-serialization-default-parser/index.js 1.77 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/components/style-rtl.css 15.5 kB 0 B
build/components/style.css 15.4 kB 0 B
build/compose/index.js 9.42 kB 0 B
build/core-data/index.js 12 kB 0 B
build/data-controls/index.js 1.27 kB 0 B
build/data/index.js 8.43 kB 0 B
build/date/index.js 31.9 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 568 B 0 B
build/dom/index.js 4.44 kB 0 B
build/edit-navigation/index.js 10.4 kB 0 B
build/edit-navigation/style-rtl.css 868 B 0 B
build/edit-navigation/style.css 871 B 0 B
build/edit-site/style-rtl.css 3.3 kB 0 B
build/edit-site/style.css 3.3 kB 0 B
build/edit-widgets/style-rtl.css 2.79 kB 0 B
build/edit-widgets/style.css 2.79 kB 0 B
build/editor/editor-styles-rtl.css 492 B 0 B
build/editor/editor-styles.css 493 B 0 B
build/editor/index.js 45.3 kB 0 B
build/editor/style-rtl.css 3.8 kB 0 B
build/editor/style.css 3.8 kB 0 B
build/element/index.js 4.45 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.52 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.js 1.74 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.54 kB 0 B
build/is-shallow-equal/index.js 711 B 0 B
build/keyboard-shortcuts/index.js 2.39 kB 0 B
build/keycodes/index.js 1.85 kB 0 B
build/list-reusable-blocks/index.js 3.02 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.js 5.12 kB 0 B
build/notices/index.js 1.69 kB 0 B
build/nux/index.js 3.27 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.44 kB 0 B
build/primitives/index.js 1.34 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/rich-text/index.js 13.7 kB 0 B
build/server-side-render/index.js 2.61 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.24 kB 0 B
build/url/index.js 4.06 kB 0 B
build/viewport/index.js 1.74 kB 0 B
build/warning/index.js 1.13 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

@alexstine
Copy link
Contributor

@ZebulanStanphill Whenever I tab through the "Post Settings" dialog, it does not read the labels.

E.g. Public, Immediately.

What does that mean? Are you missing someway to announce the labels?

E.g. Visibility, Publish.

Thanks.

@ZebulanStanphill ZebulanStanphill force-pushed the update/use-dialog-for-post-settings branch 2 times, most recently from ad5cc55 to 03c001a Compare September 16, 2020 02:45
@ZebulanStanphill ZebulanStanphill added the Needs Technical Feedback Needs testing from a developer perspective. label Sep 16, 2020
@ZebulanStanphill
Copy link
Member Author

@alexstine Are you sure that isn't just the (unfortunately) normal behavior on master? I don't have any way to test screen reader software myself at the moment, but I know the controls in "Status & visibility" were already not very accessible, hence why I created #24024 and #25170. The "Publish" and "Visibility" labels aren't even actual labels, for example. They're just <span>s.

@ZebulanStanphill ZebulanStanphill force-pushed the update/use-dialog-for-post-settings branch 7 times, most recently from 90e8bda to 8cc92dd Compare September 16, 2020 20:23
@ZebulanStanphill
Copy link
Member Author

I have discovered an issue where setting a date in the datepicker dropdown will close the entire "Post settings" dialog. I'm not entirely sure what's causing this, and I'm not sure how to fix it. Any suggestions?

Is it just not currently possible to have a dropdown inside a dialog modal? If so, then getting #24024 merged first would help us avoid that problem, though it wouldn't account for dropdowns added by plugins.

Another question: what terminology should I use in the code to refer to the document settings? Should I call it DocumentSettings or PostSettings? On the front-end the label changes depending on the post type, defaulting to "Document settings" if for some reason there's no post type label available. So I guess the document terminology would make the most sense?

@ZebulanStanphill ZebulanStanphill force-pushed the update/use-dialog-for-post-settings branch 3 times, most recently from 7625501 to 196d031 Compare September 16, 2020 22:45
@noahtallen
Copy link
Member

Maybe I'm missing something, but this feels a bit different from the latest discussion over in #20877. Do we need to continue discussing it over there? (e.g. benefits of a modal interaction, etc.)

Cool idea though; it's always nice to see experiments around this!

@ZebulanStanphill
Copy link
Member Author

@noahtallen If you want to keep discussing it, go ahead. I think the discussion in #20877 has ended up talking about two different things: current document status indicators and improvements to how document settings are accessed. I don't really think the two should be considered related in the first place, and this PR is only intended to address the latter. And the latter has been an issue long before FSE, so it needs to be addressed regardless of where #20877 goes.

My goal with this PR was to implement what I've repeatedly heard the accessibility team suggest as the most accessible solution to improving the document settings: a modal dialog.

I thought about trying the sidebar-that-acts-like-a-modal idea, but I quickly realized it would result in confusing behavior with no benefit over a standard modal dialog. The way our modal dialog is styled keeps the user focused on that dialog. And in the case of document settings, that fits, because when you're editing document settings, your focus is definitely supposed to be on those settings. There's no reason to force those settings into a limited-width area and keep the rest of the editor fully-visible... that just divides the user's attention for no reason. And when it looks like a sidebar but acts like a modal, you're breaking user expectations. So that's why I went with the approach I did.

@afercia
Copy link
Contributor

afercia commented Sep 17, 2020

setting a date in the datepicker dropdown will close the entire "Post settings" dialog.

Likely, this happens because the date picker is actually rendered in the DOM outside of the modal (I think it uses a Slot). So when you click on it, you’re actually clicking outside of the modal and the modal closes. I faced a very similar problem recently with another UI coming from another external library and luckily it was possible to specify the parent where the UI needed to be rendered. Hopefully there’s a similar configuration for the date picker too.

what do we do about the metaboxes in the "side" area? I haven't tested yet, but I assume trying to use the up/down buttons to move metaboxes from the footer area below the main WYSIWYG canvas to the metabox area in the document settings dialog won't work. To be fair, it's only very, very recently that moving them from one area to another even became possible in the block editor.

We may want to consider to entirely disable the sortable thing in the block editor. This would need a patch for core.

@afercia
Copy link
Contributor

afercia commented Sep 17, 2020

My goal with this PR was to implement what I've repeatedly heard the accessibility team suggest as the most accessible solution to improving the document settings: a modal dialog.

More accurately, the accessibility team pointed out that modal dialogs or disclosure widgets are certainly more familiar to users and easier to understand 🙂 These are well established accessible design patterns listed in the ARIA Authoring Practices and assistive technologies are built on top of these design patterns. Instead, the whole concept of "sidebar" isn't standardized anywhere, can't be communicated semantically, and the expected interaction is unclear at the point some implementations resort to implementing a modal behavior anyways.

To me, also the UI proposed in #20877 is basically a modal dialog, though it looks different. Functionally, it would need to have a modal behavior thus it would need to be semantically a dialog. The advantage of this PR compared to #20877 is that the expandable panels are easier to use than a tabbed interface.

I thought about trying the sidebar-that-acts-like-a-modal idea, but I quickly realized it would result in confusing behavior with no benefit over a standard modal dialog.

I'd agree and I'd say it applies also to the new Inserter where a modal behavior was added last minute to remediate to the new Sidebar pattern. Further investigation on the Inserter from an a11y perspective will happen on #24975.

Generally, and I think I can speak on behalf of the accessibility team, anything that explores patterns that move away from the concept of "sidebar" is welcome. Testing this PR, the separation between document-level (post) settings and block settings the Inspector) is finally clear. Not to mention the UI is cleaner and easier to use.

There are probably some things to better consider for example, as mentioned above, the "side" meta boxes. Generally, I'd tend to think most of the post settings are used once or very few times in the entire life of a post. These are settings that don't need to be always displayed in the sidebar and a modal dialog feels more appropriate to me (also, as Zeb said, because it focuses on a specific task).

However, there are at least a couple things that need to be considered:

  • Plugins can add their own panels to the "document" sidebar: depending on the use case, moving these plugin-panels to the modal may not be that appropriate because they would be way less discoverable.
  • The "Move to trash" button: I'm not sure the modal dialog is an appropriate placement for this button. Although it's typically used once per post, I'd tend to think "Move to trash" should be always visible in the editor UI. Placing it within the modal resembles the Themes "Delete" button which is only available in the Theme details dialog and it's a known discoverability issue. Worth noting this applies also to Rethinking the "Current Document" display and settings #20877, where the "Move to trash" button is a bit buried down within the popup and the tabs.

Screenshot to illustrate the "Move to trash" button and a panel added by a plugin (cough) within the modal dialog in this PR:

Screenshot 2020-09-17 at 14 28 03

From a technical perspective, I see some odd behaviors with tabbing and focus. For example, I couldn't find a way to tab to the "Use a secure password" input field within the visibility popover. Maybe this might be fixed in #24024.

Screenshot 2020-09-17 at 14 23 06

@ZebulanStanphill
Copy link
Member Author

We may want to consider to entirely disable the sortable thing in the block editor. This would need a patch for core.

Are you suggesting we "lock" the position of metaboxes in the block editor? Does that mean we'd be showing them in alphabetical order or something like that? Is there any way to tell the difference between a "wide" (footer area) metabox and a "skinny" (side area) metabox?

The "Move to trash" button: I'm not sure the modal dialog is an appropriate placement for this button. Although it's typically used once per post, I'd tend to think "Move to trash" should be always visible in the editor UI. Placing it within the modal resembles the Themes "Delete" button which is only available in the Theme details dialog and it's a known discoverability issue. Worth noting this applies also to #20877, where the "Move to trash" button is a bit buried down within the popup and the tabs.

Depending on how you look at it, "Move to trash" might not even be considered a "document setting". I agree it should be exposed more prominently. I'm thinking we could have it always visible in the top toolbar, and add a confirmation dialog to handle misclicks. How does that sound? I think I could create a separate PR for that, if you want.

Plugins can add their own panels to the "document" sidebar: depending on the use case, moving these plugin-panels to the modal may not be that appropriate because they would be way less discoverable.

Yeah, depending on the plugin and the functionality, it might make more sense to give them their own button and modal or dropdown in the top toolbar, rather than keep them in the document settings modal. And some, perhaps, might even belong in their own sidebar.

In the case of that Yoast SEO example, that thing feels less like a settings section and more like a reminder/status-indicator. It feels like a pre-publish check, and sure enough, Yoast adds a pre-publish check with the exact same content. So... does that particular section even need to exist in the first place? I feel like it's compensating for the Yoast SEO sidebar apparently not being discoverable enough (and perhaps enabling text labels in the top toolbar by default is the solution here, though that's a whole other discussion).

Maybe we ought to consider adding a "document status/overview" popover/modal/something that lists out important details of the document (e.g. visibility, publish state, SEO status), with "Edit" links to jump to the parts of the UI dedicated to changing those details. Maybe we could even combine it with the "Content structure" tool (the one that shows word count and stuff like that)? Just throwing ideas out there.

@afercia
Copy link
Contributor

afercia commented Sep 17, 2020

Are you suggesting we "lock" the position of metaboxes in the block editor? Does that mean we'd be showing them in alphabetical order or something like that? Is there any way to tell the difference between a "wide" (footer area) metabox and a "skinny" (side area) metabox?

We need to distinguish to different things:

  • reordering meta boxes within the same sortable area
  • reordering meta boxes across different sortable areas

The latter already has some check for the case when there's just one sortable area. The relevant code is in WordPress src/js/_enqueues/admin/postbox.js, see handleOrderBetweenSortables(). However, the specific sortable areas can only be distinguished by their ID. We can do that reliably for the sortable areas that come from core but worth considering plugins can add their own sortable areas (for example Advanced Custom Fields does add at least one of them if I remember correctly). In short, the whole thing is a bit messy 🙂 However, I'm wondering if reordering meta boxes in the block editor does make sense in the first place, as meta boxes are to be considered legacy.

Regarding the other considerations in your previous comment, I totally agree there should be a better separation between things that are real "post settings" and other things that are more status / overview information related to the post content. A place that better suits information about the content is certainly the "Content structure" tool, as you mentioned.

Maybe this should be considered a bit more in depth by Design, focusing on the specific task to introduce better separation. Thoughts?

@paaljoachim
Copy link
Contributor

paaljoachim commented Sep 28, 2020

Testing the PR. I really like the explorations going on here! One has a chance to rethink current Gutenberg patterns!

Sharing thoughts as they arise along the way...

Current top right bar:
Screen Shot 2020-09-28 at 09 18 10

PR proposal top right bar:
Screen Shot 2020-09-28 at 09 19 21

Should the Switch to draft, Preview and Update button be all the way to the right? As in move the 3 dot Option icon and Block Inspector icon. I am saying this as I wonder if the Update button should be the last clickable item in the bar. The buttons also have most weight and one would also gather the icons next to each other.

Moving-top-bar-icons-around

Current method clicking the gear icon opens Block Settings in the sidebar.

PR proposal method clicking the gear icon opens the Page Settings in a dialog box. It feels to be free floating in the middle of the screen. I instantly want to dock it to the right side as is seen in the current sidebar method. As it is so very different from how the Options screen is seen.

Screen Shot 2020-09-28 at 09 33 00

It would be nice to keep the consistency when clicking the Options icon and the Settings icon. That they both open in the same area. Or another option could be to do something like this (I still want to push the settings all the way to the right edge):
Page-Settings-drop-down

As it would open similar to how List View, Details and Modes open. Making it use an existing Gutenberg UI pattern.

Page and Block settings

What is nice with the current approach is that it gathers the Page and Block settings into one panel area. Using tabs system to go between Page <-> Block. Creating an association between them.

The PR proposal method. The Block Inspector and Page settings are split a part creating a greater distinction between the two. Block inspector is still in the sidebar and the Page settings is brought out from the sidebar. It also adds another icon to the top bar.

What if Block and Page settings were like so.

Page-block-settings-drop-down2

We then have the Settings gear and beside it the Options 3 dots icon. I moved the Switch to draft, Preview and Update buttons to the right of the icons. I removed the block icon. I was now also able to move the settings all the way to the right, so it now looks somewhat similar to the current method of opening the settings in the sidebar.

@ZebulanStanphill
Copy link
Member Author

The block inspector, from a semantic perspective, should never have been placed in the same tabbed sidebar as the document settings. And it shouldn't be opened from the top bar, either. The accessibility team has requested that the block inspector be accessed from a control near the block itself, so my plan (for a future PR) is to move the block inspector toggle to the block toolbar.

I didn't want to do that in this PR because there's already so many changes in this one. And I also didn't want to open another PR to make the block inspector change that is branched off of this one until I've fixed some of the issues with this one... otherwise I'll have to constantly rebase it everytime I change this PR.

The button order is something that could use some rethinking, but for the sake of keeping this PR small, I can just move the "Document settings" button next to the "Block inspector" button for now to keep things relatively similar to master and try a full reorganization in a future PR.

I did consider putting the document settings in a simple dropdown, but the issue is that there's a lot of document settings, and I figured any simple dropdown would be too small. By using a full dialog, you get the ability to use as much space as necessary, and I figure that we could update the Document Settings to look like this on desktop:

image

This is a mockup for an Options modal redesign from #24965 (comment), but if the design ends up working there, why not use it again here?

@paaljoachim
Copy link
Contributor

paaljoachim commented Oct 1, 2020

The accessibility team has requested that the block inspector be accessed from a control near the block itself, so my plan (for a future PR) is to move the block inspector toggle to the block toolbar.

Are you thinking something similar to this following mockup?
Block-Inspector-icon-toolbar


The button order is something that could use some rethinking, but for the sake of keeping this PR small, I can just move the "Document settings" button next to the "Block inspector" button for now to keep things relatively similar to master and try a full reorganization in a future PR.

Are you thinking something like this mockup?
Moving-top-bar-icons-around

@ZebulanStanphill
Copy link
Member Author

@paaljoachim In my first statement there, I was suggesting just moving the button to the block toolbar (and changing its icon to a cog again, since that would fit better in the context of the block toolbar). I think that could be a definite a11y/discoverability improvement that the design team would be okay with.

Ultimately, the block inspector should also be made to act like a modal, meaning it should capture focus and close when it is unfocused, similar to most dropdowns in the UI (as well as the Options modal). Perhaps it should even work like the Options modal and look/function as a dialog. But I know that would be a controversial move, so I'm not trying to do that yet.

As for the second mockup, that's pretty much exactly what I had in mind.

@paaljoachim
Copy link
Contributor

The changes you propose in this PR is probably something that should be discussed during the Open Floor of a Core Editor Meeting. As there are likely many different view points that need to come forth.

Base automatically changed from master to trunk March 1, 2021 15:44
@ZebulanStanphill
Copy link
Member Author

I'm no longer pursuing this redesign, partially because I no longer have the time to pursue it, partially because the UI has taken a different direction since this PR, and partially since I'm no longer convinced all the changes in this PR are for the better.

If anyone is still interested in a modal-like settings sidebar, EditorsKit has something kinda similar in the form of its "Use Sidebar as Moveable Modal" option.

@ZebulanStanphill ZebulanStanphill deleted the update/use-dialog-for-post-settings branch July 28, 2022 00:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Document Settings Document settings experience [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Needs Accessibility Feedback Need input from accessibility Needs Design Feedback Needs general design feedback. Needs Technical Feedback Needs testing from a developer perspective.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants