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

Potential ListView updates from the navigation list view experiment #47039

Closed
5 tasks
draganescu opened this issue Jan 10, 2023 · 8 comments
Closed
5 tasks

Potential ListView updates from the navigation list view experiment #47039

draganescu opened this issue Jan 10, 2023 · 8 comments
Labels
[Feature] List View Menu item in the top toolbar to select blocks from a list of links. Needs Technical Feedback Needs testing from a developer perspective. [Type] Enhancement A suggestion for improvement.

Comments

@draganescu
Copy link
Contributor

draganescu commented Jan 10, 2023

An experiment named "Off canvas navigation editor" ran. It implements a way to manage a menu using a tree view in the inspector controls of the navigation block. The calls for testing offered a very good response in terms of this experiment making menu editing better, easier and clearer.

As a result of the validation of the experiment the new feature can be moved out of experiment mode. However, to run the experiment some experimental code was created. The core of this experimental code lives in a temporary __ExperimentalOffCanvasEditor component.

__ExperimentalOffCanvasEditor is a copy of the ListView component. The intent of this component was to allow of experimental code to be committed for the experiment to be validated. It was meant to be temporary and to highlight what ListView would need to make the experiment possible.

__ExperimentalOffCanvasEditor introduces the following things:

Two new props:

  • optional block selection, a flag named selectBlockInCanvas which determines if clicking on a list view leaf should move selection to the canvas and select the respective block
  • optional more menu, a prop named LeafMoreMenu which allows one to pass a different more menu for a leaf of the list view

New list view features:

  • an edit button, which acts as a mechanism to select the block and access its block inspector, without moving the focus in canvas. This feature works in tandem with the selectBlockInCanvas flag, it only makes sense if selecting the block is disabled.
  • adding an appender at the bottom of the list view, this adds a appender which invokes a quick inserter for adding items at the top of the block tree. This feature is not scoped and does not reside currently under a flag.

A few style updates:

  • spacing updates: because placing the list view in a block inspector does not benefit from an expanding container (like the list view container is) we use built in horizontal scrolling for deeply nested blocks. To limit the need for horizontal scrolling some updates have been added to spacing of the items in the temporary __ExperimentalOffCanvasEditor component.
  • a smaller "lock" icon, same as above due to new space constraints the icon for locked blocks was made smaller.

LinkUIupdates, which are part of the temporary __ExperimentalOffCanvasEditor but should be ported into the actual LinkUI component. The "Off canvas navigation editor" produced a number of improvements outside the scope of the experiment to the PageList block, to the Navigation block and to LinkUI. The updates to LinkUI are the only ones to not exist outside of the experiment but they provide value outside of it.

Having a ListView and an OffCanvasEditor component is, in my opinion, not needed. To properly implement the new one a refactoring of ListView would be needed to enable the reuse of most of its inner components. Looking at the above additions it seems too much.

I propose the following updates to ListView as the next steps of promoting the "Off canvas navigation editor" experiment to a feature, each with its own PR and discussion:

  • implement the style updates to ListView
    The style updates make the ListView use space better. It is already a bit weird hot as you expand leaves the list view sidebar keeps getting bigger. Most of these updates, including the smaller lock icon, use screen real estate better and lower this effect.

  • implement in ListView the selectBlockInCanvas and the new block "edit" button on the list view leaf in tandem.
    This mode of editing content using a tree view scoped to the local context of the current selection has the potential to grow into a better way to handle locked block templates, in a way in which makes the content editable but not the structure or the styling. The ability to disable block selection will come in handy to allow the focus to remain on the tree representation of the blocks.

  • implement an optional appender at the bottom for ListView
    The list view had an appender. It was later removed, now it's back. Originally the list view appender was for every leaf. Due to ergonomics and the improved drag and drop experience it seems using only one bottom appender is good enough. There is a worth in having a discussion if this appender should be optional of if the normal list view use for deeply nested layouts doesn't benefit of just having an appender right there.

  • implement the possibility of overriding the more menu for ListView items
    The default more menu for blocks, available as a dot menu in the block toolbar, is very restrictive in terms of customization. This is an update that allows for flexibility in terms of providing contextually useful actions and not fighting the system (e.g. a make template part on a navigation item makes no sense). This is - for me - the most debatable addition, the other path I can think of is to simply improve how the more menu can be customized.

  • Port over any updates to LinkUI that are not scoped to the temporary
    __ExperimentalOffCanvasEditor.
    These updates enhance LinkUI 's ability to cope with being invoked in varied contexts.

This is not a definitive to do list, nor is it the only way to promote the experiment to feature. My point is mainly that what has surfaced as ListView updates for the navigation experiment are actually useful features, improvements, of the ListView component. This idea can be challenged.

Each task should be done in a standalone PR to make sure that adding the features and updatas to ListView fully take into consideration the restrictions that this component assumes (in terms of accessibility, intent and usefulness) so that we don't end up bloating a component for the needs of one block (to here I'd like to re-mention that editing nested blocks is a problem that not only the navigation block has).

cc @talldan

@draganescu draganescu added [Type] Enhancement A suggestion for improvement. Needs Technical Feedback Needs testing from a developer perspective. [Feature] List View Menu item in the top toolbar to select blocks from a list of links. labels Jan 10, 2023
@talldan
Copy link
Contributor

talldan commented Jan 12, 2023

I think I've seen a few more things in the codebase than listed here.

The style updates make the ListView use space better. It is already a bit weird hot as you expand leaves the list view sidebar keeps getting bigger. Most of these updates, including the smaller lock icon, use screen real estate better and lower this effect.

There are some styling issues in the off-canvas editor that would be great to resolve so that they're not brought over to ListView.

I also don't think the horizontal scrollbar is the best solution. Several options for this have been explored in List View in the past. Possibly we should explore resizable sidebars now that there's a precedent.

It is already a bit weird hot as you expand leaves the list view sidebar keeps getting bigger.

It's probably a bug. 😄

implement in ListView the selectBlockInCanvas and the new block "edit" button on the list view leaf in tandem.

implement the possibility of overriding the more menu for ListView items

These are both things where I think it'd be good to take time in exploring how List View can be made more extensible. There have been lots of requests for adding additional controls to List View, and I'd hate to see a proliferation of props for controlling these things.

What if by default, List View doesn't have any additional controls (no block settings menu). The persistent list view implementation is something like this pseudocode:

<ListView
    blockControls={ <ListViewBlockSettingsMenu /> }
/>

And the off canvas editor is like this:

<ListView
    blockControls={
        <>
            <OffCanvasEditBlockButton />
            <OffCanvasSettingsMenu />
        </>
    }
    disableBlockSelection
/>

implement an optional appender at the bottom for ListView

I wouldn't like to see a 'showAppender' prop. For me, the appender hasn't really proved itself, I think it's of limited use if it only adds to the end at the root level. Probably the most useful place for it would be the post editor, but it won't be particularly useful in the site editor, and won't work in the widget editor (where it'd be appending widget areas). I think there could be more effort put into alternative solutions like a sibling inserter and insert before/after

Port over any updates to LinkUI that are not scoped to the temporary __ExperimentalOffCanvasEditor.

This needs a bit more context. I'm guessing it's unrelated to List View? List View doesn't have a Link UI.

@annezazu
Copy link
Contributor

I added this to the 6.2 board but it seems like this might be for later. If that's the case, can punt it to the 6.3 column of the board! Just let me know.

@talldan
Copy link
Contributor

talldan commented Jan 25, 2023

The off canvas editor is currently one of the experimental APIs to audit, and I think the best thing for 6.2 might be to keep it experimental and use the new experiment locking system introduced in #46131. It hopefully shouldn't be complex to make a single component an experiment. This way there's less pressure for everyone and no issues with experimental APIs being public.

Then for 6.3 I think it'd be good to explore making List View more extensible so that it can be configured with additional controls like those the off canvas experiments.

@scruffian
Copy link
Contributor

I wouldn't like to see a 'showAppender' prop. For me, the appender hasn't really proved itself, I think it's of limited use if it only adds to the end at the root level. Probably the most useful place for it would be the post editor, but it won't be particularly useful in the site editor, and won't work in the widget editor (where it'd be appending widget areas). I think there could be more effort put into alternative solutions like a sibling inserter and insert before/after

@talldan What do you think about adding a Slot at the end of the List View so that we could optionally add an appender to it using the Slot/Fill pattern?

@talldan
Copy link
Contributor

talldan commented Feb 17, 2023

@scruffian I've changed my mind 😄 A showAppender prop is I think the right way to go if it's locked using the new experimental API locking system. That's the easiest option.

Slot/Fill is usually good when there's more than one thing to be rendered, but I think it'll only ever be the appender that appears in this position.

@scruffian
Copy link
Contributor

I have broken this into three sub-issues:
#46992
#46993
#46991

@andrewserong
Copy link
Contributor

Just for visibility in case there's any overlap, I've written up a separate issue listing out some of the list view usability issues I'd like to have a go at over in #49563 🙂

@scruffian
Copy link
Contributor

I think we can close this one now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] List View Menu item in the top toolbar to select blocks from a list of links. Needs Technical Feedback Needs testing from a developer perspective. [Type] Enhancement A suggestion for improvement.
Projects
Status: Done
Development

No branches or pull requests

5 participants