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

Try a toolbar for the block navigator #21372

Closed
wants to merge 6 commits into from

Conversation

talldan
Copy link
Contributor

@talldan talldan commented Apr 3, 2020

Description

#18014 attempted to introduce inline block movers for the Block Navigator, but this proved to be quite difficult.

This PR tries a different approach, introducing a mini-toolbar for the navigator (currently added to the Navigation Menu Page) with a few actions - Move, Duplicate, Delete (we'll need some icons for the last two, for now I've gone with a couple that are available).

This approach is a bit simpler, since it's a separate component that doesn't involve changing the navigation list itself. It's also a bit more scalable, the toolbar has more space for other buttons. It might also be an option to remove the block appenders from the list and use a toolbar button instead.

At the moment the toolbar is fairly basic, but there are a few improvements that could be made:

  • Animated blocks when they move
  • Make the toolbar and navigation list a single tab stop for keyboard users
  • Improved icons

How has this been tested?

  1. Go to the navigation menu page.
  2. Try using the toolbar in the Navigation Structure panel.

Screenshots

Screenshot 2020-04-03 at 3 07 08 pm

block-navigator

Types of changes

New feature (non-breaking change which adds functionality)

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.

@talldan talldan added [Type] Enhancement A suggestion for improvement. Needs Design Feedback Needs general design feedback. [Feature] List View Menu item in the top toolbar to select blocks from a list of links. labels Apr 3, 2020
@talldan talldan self-assigned this Apr 3, 2020
@github-actions
Copy link

github-actions bot commented Apr 3, 2020

Size Change: +477 B (0%)

Total Size: 846 kB

Filename Size Change
build/block-editor/index.js 106 kB +496 B (0%)
build/block-editor/style-rtl.css 10.2 kB +43 B (0%)
build/block-editor/style.css 10.2 kB +43 B (0%)
build/block-library/index.js 112 kB -1 B
build/components/index.js 198 kB -141 B (0%)
build/edit-navigation/index.js 3.58 kB +39 B (1%)
build/editor/index.js 43.3 kB -1 B
build/element/index.js 4.65 kB +1 B
build/rich-text/index.js 14.8 kB -2 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.02 kB 0 B
build/annotations/index.js 3.62 kB 0 B
build/api-fetch/index.js 4.08 kB 0 B
build/autop/index.js 2.82 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 6.24 kB 0 B
build/block-directory/style-rtl.css 760 B 0 B
build/block-directory/style.css 761 B 0 B
build/block-library/editor-rtl.css 7.13 kB 0 B
build/block-library/editor.css 7.13 kB 0 B
build/block-library/style-rtl.css 7.19 kB 0 B
build/block-library/style.css 7.19 kB 0 B
build/block-library/theme-rtl.css 683 B 0 B
build/block-library/theme.css 685 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 57.7 kB 0 B
build/components/style-rtl.css 16.9 kB 0 B
build/components/style.css 16.9 kB 0 B
build/compose/index.js 6.66 kB 0 B
build/core-data/index.js 11.4 kB 0 B
build/data-controls/index.js 1.29 kB 0 B
build/data/index.js 8.43 kB 0 B
build/date/index.js 5.47 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 569 B 0 B
build/dom/index.js 3.1 kB 0 B
build/edit-navigation/style-rtl.css 485 B 0 B
build/edit-navigation/style.css 485 B 0 B
build/edit-post/index.js 28.2 kB 0 B
build/edit-post/style-rtl.css 12.5 kB 0 B
build/edit-post/style.css 12.5 kB 0 B
build/edit-site/index.js 10.8 kB 0 B
build/edit-site/style-rtl.css 5.25 kB 0 B
build/edit-site/style.css 5.24 kB 0 B
build/edit-widgets/index.js 8.33 kB 0 B
build/edit-widgets/style-rtl.css 5 kB 0 B
build/edit-widgets/style.css 5 kB 0 B
build/editor/editor-styles-rtl.css 428 B 0 B
build/editor/editor-styles.css 431 B 0 B
build/editor/style-rtl.css 3.27 kB 0 B
build/editor/style.css 3.27 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.32 kB 0 B
build/format-library/style-rtl.css 502 B 0 B
build/format-library/style.css 502 B 0 B
build/hooks/index.js 2.13 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/is-shallow-equal/index.js 711 B 0 B
build/keyboard-shortcuts/index.js 2.51 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.12 kB 0 B
build/list-reusable-blocks/style-rtl.css 226 B 0 B
build/list-reusable-blocks/style.css 226 B 0 B
build/media-utils/index.js 5.29 kB 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/index.js 3.4 kB 0 B
build/nux/style-rtl.css 616 B 0 B
build/nux/style.css 613 B 0 B
build/plugins/index.js 2.67 kB 0 B
build/primitives/index.js 1.49 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.84 kB 0 B
build/server-side-render/index.js 2.67 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.28 kB 0 B
build/url/index.js 4.02 kB 0 B
build/viewport/index.js 1.84 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

@draganescu
Copy link
Contributor

I love this idea!

@mtias
Copy link
Member

mtias commented Apr 3, 2020

I think this has merit, it'd be a sensible place to have an "Edit" label that would make the attributes editable (labels). What it makes me think, though, is that it seems it could possibly just replicate the child block toolbar items entirely (the link, submenu creation, etc).

@ZebulanStanphill ZebulanStanphill added the Needs Accessibility Feedback Need input from accessibility label Apr 3, 2020
@youknowriad youknowriad mentioned this pull request Apr 7, 2020
22 tasks
@karmatosed
Copy link
Member

@talldan I don't seem able to test the latest version of this, but happy to see if this is me. I just get a splash of the screen then it goes away. If you can get it working, happy to review a video.

@noisysocks noisysocks added the [Priority] High Used to indicate top priority items that need quick attention label Apr 15, 2020
@talldan
Copy link
Contributor Author

talldan commented Apr 16, 2020

@karmatosed It might be related to this issue - #21633

The panel looks open while it's actually collapsed.

@karmatosed
Copy link
Member

@talldan I sadly don't get a panel to open, just white screen. Are you able to run this in gutenberg.run as I also can't there? It very much might be me or browser-based. I would love to test for you though.

@talldan talldan marked this pull request as draft April 17, 2020 06:52
@talldan talldan added the [Status] In Progress Tracking issues with work in progress label Apr 17, 2020
@talldan talldan force-pushed the try/alternate-navigator-movers branch from 6683673 to dffd978 Compare April 17, 2020 07:01
@talldan
Copy link
Contributor Author

talldan commented Apr 17, 2020

@karmatosed You do need to create a menu using the existing menu page first before going into the experimental page. At the moment the experimental page doesn't have an implementation for creating menus.

I just rebased the branch, and it does seem to be working ok.

@talldan talldan force-pushed the try/alternate-navigator-movers branch from f346b1f to d5318ac Compare April 24, 2020 07:34
@noisysocks noisysocks removed the [Priority] High Used to indicate top priority items that need quick attention label Apr 26, 2020
@karmatosed
Copy link
Member

@talldan just looking at the video, one thing that strikes me is a disconnect from the item being moved. This could get worse as the tree grows with items. I think having movers beside would, in this case, be more ideal because of that. I know there was an issue to bring that within the block navigator and I maybe think doing that would be useful. That said, perhaps if there is a limit on the height of the navigator area, it wouldn't be a growing problem. I am wary though of this having different interactions to the navigator interface in other situations.

#18014 specifically is one I think worth noting here of a way to have the movers in the actual interface.

@talldan
Copy link
Contributor Author

talldan commented Apr 28, 2020

@karmatosed Yeah, I can definitely see that. I can definitely try to make some progress on #18014, though it has been difficult so far getting that to work for screen reader/keyboard only users.

One thing that concerns me is that there are quite a few different block-related actions that are proposed for the navigator and I'm not completely clear how they all fit together.

So far there's editing (#20748), deleting (#10984) and a few others like being able to move blocks in and out of nested content (something that seems to be impossible for keyboard only users in the block editor as a whole).

So that's one thing I had in mind when considering this toolbar.

@karmatosed
Copy link
Member

though it has been difficult so far getting that to work for screen reader/keyboard only users.

Maybe this is something we can collaborate on and get a solution for? Looping in @enriquesanchez too maybe get eyes on that too.

@enriquesanchez
Copy link
Contributor

Maybe this is something we can collaborate on and get a solution for? Looping in @enriquesanchez too maybe get eyes on that too.

@karmatosed @talldan Happy to help. I just tested #18014 and I'm now aware of the challenges on that PR. Leaving comments over there to keep the conversation focused on the PR.

@adamziel
Copy link
Contributor

adamziel commented Apr 30, 2020

I took it for a spin and it works pretty well (aside of the keyboard navigation issues already mentioned in previous comments). One thing I noticed is that clicking on a menu item in "Navigation structure" moves the focus over to "Navigation menu". This is because we selectBlock on click. Making the menu item editable in #21948 should fix this behavior.

@talldan
Copy link
Contributor Author

talldan commented May 7, 2020

Closing this as it seems as though keeping the block UI inline in the navigator is the preferred design - see #22089 for details.

@talldan talldan closed this May 7, 2020
@talldan talldan deleted the try/alternate-navigator-movers branch May 7, 2020 03:49
@shaunandrews
Copy link
Contributor

Huh, I just discovered this PR after independently creating the following GIF:

navigation-list-with-toolbar

FWIW, I think this is a very interesting option and something I'd like to see us explore again.

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 Accessibility Feedback Need input from accessibility Needs Design Feedback Needs general design feedback. [Status] In Progress Tracking issues with work in progress [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants