Try/publish in site editor #51408
Merged
Try/publish in site editor #51408
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Size Change: +1.4 kB (0%) Total Size: 1.4 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 2a13222. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5317296705
|
|
It seems quite likely that one would want to edit the status from both the Page Details panel, and the Page Inspector in the Editor. In either case, status, visibility, and password protection don't seem like details you will edit with all that much frequency. These assumptions make me wonder whether a modal could work well for this edit UI. The same result is produced, regardless of entry point: 
It could even work as a drilldown, if we wanted to go in that direction. What do you think? |
|
@jameskoster I updated PR to use modal initially just triggered via page inspector. One bit of jank is that it doesn't "save" the status change until a user hits save . We could save but would need to prompt to save other details before. Have a play and see what you think. |
|
Nice. I don't actually mind the jank, at least it's consistent with the post editor / other meta changes. We might need to include the calendar for all statuses except Draft and Pending, otherwise you cannot update the publish date. |
SaxonF
force-pushed
the
try/publish-in-site-editor
branch
from
eb424b2 to
0d25392
Compare
SaxonF
removed request for
ajlende,
noahtallen,
ObliviousHarmony,
kevin940726,
tellthemachines,
fluiddot,
fabiankaegy and
t-hamano
kevin940726
reviewed
Jun 19, 2023
packages/edit-site/src/components/sidebar-edit-mode/page-panels/page-status.js
Outdated
Show resolved
Hide resolved
packages/edit-site/src/components/sidebar-edit-mode/page-panels/page-status.js
Outdated
Show resolved
Hide resolved
packages/edit-site/src/components/sidebar-edit-mode/page-panels/page-status.js
Outdated
Show resolved
Hide resolved
packages/edit-site/src/components/sidebar-edit-mode/page-panels/page-status.js
Outdated
Show resolved
Hide resolved
packages/edit-site/src/components/sidebar-edit-mode/page-panels/page-status.js
Outdated
Show resolved
Hide resolved
packages/edit-site/src/components/sidebar-navigation-screen-page/style.scss
Outdated
Show resolved
Hide resolved
| // Anchor the popover to the middle of the entire row so that it doesn't | ||
| // move around when the label changes. | ||
| anchor: popoverAnchor, | ||
| placement: 'bottom-end', |
There was a problem hiding this comment.
The popover doesn't have a descriptive name for screen readers, users might not even know that they land in a popover. Maybe we should add aria-label or aria-labelledby here.
The other thing is that Popover traps the focus but it doesn't set the dialog role. This could be a bigger problem in general of the component design and I personally think it's an accessibility bug. We could set a role: 'dialog' here but I'm not sure if it's the right move. Possibly better to consult an accessibility expert on this.
It's still usable thought just a bit confusing so I don't think this should be blocking 👍
There was a problem hiding this comment.
Added an aria-label for the time being since I don't want to modify InspectorPopoverHeader or PublishDateTimePicker as part of this PR to add ID to title.
The other thing is that Popover traps the focus but it doesn't set the dialog role. This could be a bigger problem in general of the component design and I personally think it's an accessibility bug. We could set a role: 'dialog' here but I'm not sure if it's the right move. Possibly better to consult an accessibility expert on this.
Let's create a separate issue referencing the Dropdown and Popover components in wordpress/components to review any accessibility concerns. Modal has a title prop that uses aria-labelledby.
kevin940726
added
the
[Feature] Site Editor
|
@kevin940726 Thanks for the review! I'll fix up your suggestions first thing tomorrow and hopefully we can get this merged later in the day :) |
|
@kevin940726 ready for a re-review please! |
kevin940726
approved these changes
Jun 20, 2023
packages/edit-site/src/components/sidebar-edit-mode/page-panels/page-status.js
Outdated
Show resolved
Hide resolved
sethrubenstein
pushed a commit
to pewresearch/gutenberg
that referenced
this pull request
Jul 13, 2023
* add required inputs * schedule date * add password and fix a few things * re-add status modal * update change status modal * revert in sidebar status change * refine a few status details * fix merge issues * fix status label * use null for status date * handle empty date * change to add fields instead of modal * default publish not draft * remove style import * add required inputs * schedule date * add password and fix a few things * re-add status modal * update change status modal * revert in sidebar status change * refine a few status details * fix merge issues * fix status label * use null for status date * handle empty date * change to add fields instead of modal * default publish not draft * remove style import * remove css * only autofocus password field if empty * aria label on status popover * remove end line * create radio with help component * adjust to use radiocontrol and hide password * radio label alignment * add form wrapper * aria label
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Adds ability to set status, publish date and password in site editor.
Why?
Resolves #49873
How?
WIP.
Testing Instructions
pagesand select a page to editTesting Instructions for Keyboard
Screenshots or screencast