Site Editor: Persistent List View #28637
Merged
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: +845 B (0%) Total Size: 1.39 MB
ℹ️ View Unchanged
|
Copons
changed the title
Copons
added
[Feature] List View
mtias
reviewed
Feb 2, 2021
Copons
added
Needs Accessibility Feedback
|
I've opened this for review and added a few points of discussion to the PR description. |
Copons
requested review from
vindl,
mattwiebe,
noahtallen,
Addison-Stavlo,
david-szabo97,
jeyip and
jameskoster
|
One nice thing to add would be an indication of the currently-selected-blocks children:
This can be a helpful visual cue for when a block has many innerblocks. Also wondering if we need the padding around the items? full-width.mp4We might give the canvas more room by eliminating it. On a similar note, the icons feel especially hefty now. I wonder if we might shrink them down a bit here, and tweak the spacing slightly? Here's 16px icons, and 8/12px padding on the buttons:
|
Copons
changed the title
|
I made some styling adjustments:
Additional css classes will be required to achieve this though:
|
Copons
force-pushed
the
add/fse-persistent-block-navigator
branch
2 times, most recently
from
64e8807 to
82527d9
Compare
|
@jameskoster I don't think we should have a fixed width: the
On the other hand, with an automatic width, if we have a very long block name, or a very deeply nested block, we might end up with an exceptionally large sidebar. Given how the style changes need to be tested carefully, as they risk affecting the |
|
I am open to moving general styling to a separate issue, but I think we should get the width sorted in this PR as that feels foundational to the overall usability of this feature. I do not think the width should vary from page to page, or based on which blocks / nesting exists within the current document. Like you said, there's potential for a large variance which would feel quite disorientating to use. So, options... I see only two at this point:
Any other ideas? |
InterfaceSkeleton's secondarySidebar prop always render something unless it's falsy, so passing a component that renders null is not enough. By handling the Inserter and List View sidebars visibility from the parent component, I had to also move the List View shortcut so that it's always available, not just when the sidebar is visible. Props to @jeyip for catching this!
Props @Addison-Stavlo ✨
Copons
force-pushed
the
add/fse-persistent-block-navigator
branch
from
d812cec to
e907eab
Compare
|
Thanks for the suggestion @Addison-Stavlo! Pushed your change, and it seems the focus works as expected.
|
Addison-Stavlo
approved these changes
Mar 1, 2021
There was a problem hiding this comment.
Awesome! That works well for me and everything else looks good.
As noted we can follow up on the other design and UX/UI uncertainties, but this seems in a good place to start testing out the feature!
Thanks for all your work on this @Copons !
This was referenced Mar 2, 2021
|
I noticed the following confusing behavior. When you use arrow keys in List View and press Screen.Recording.2021-03-03.at.17.03.02.mov |
@gziolo Yes, we are (painfully) aware of it: #29467 😄 We talked at lenght about it in this PR too (at this point all buried in the hidden comments), we even went back and forth with some visual experiment, and eventually decided to move the discussion to a follow up, to avoid stalling this PR. Designers are aware of it too, and are thinking of a nice solution as we speak. 👍 |
This was referenced Mar 10, 2021
25 tasks
7 tasks
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.
Description
Fixes #27395
Convert the List View dropdown into a persistent sidebar occupying the same interface skeleton slot as the Inserter (
secondarySidebar).To achieve this, a number of widespread changes have been made:
components/secondary-sidebarfolder containing both the Inserter and the List View file, making the Site Editor'seditor.jseasier to read.listViewPanelreducer (and relative action and selector) in the same vein as those for the inserter panel and navigation sidebar. These three reducers are interconnected: only one can be open at any time, so the action that opens one will automatically close the others.listViewicon containing the List View icon, which at this point would have been hardcoded and repeated in 3 different files. Also replace those hardcoded icons with the new one.BlockNavigationDropdownin the Site Editor header with a custom implementation that would fill thesecondarySidebarwith theBlockNavigationTreecomponent, displaying the entire blocks hierarchy of the content, as opposed to the dropdown, which only shows the currently selected nested hierarchy (e.g. if the site title block is selected, the dropdown would only show up to the header template part).BlockNavigationTreeitself).highlightBlocksOnHoverprop to theBlockNavigationcomponent to support highlighting the corresponding block in the canvas on hover and focus.TODO
Stuff to discuss
The keyboard navigation.
This is complicated, and any changes will affect the original
BlockNavigationcomponent itself.What makes things awkward is that selecting a block in the
BlockNavigationmoves the focus to the canvas, and selecting blocks in the canvas will change theBlockNavigationselected item.A common scenario would be:
In other words: once the user selects an item in the sidebar, the vertical traversal changes.
Whereas one would expect that traversing the block list is just a matter of going up/down, traversing the canvas is a different matter.
Horizontally aligned blocks might want an horizontal movement instead, practically blocking the traversal.
How has this been tested?
Screenshots
(Ignore the unstyled canvas, it's an unrelated issue)
Types of changes
New feature (non-breaking change which adds functionality)
Checklist: