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

Proof of Concept: Improve Block Toolbar Semantics/Accessibility #53779

Closed
wants to merge 25 commits into from

Commits on Sep 6, 2023

  1. Move Selected Block Tools into the header toolbar in the DOM but pres…

    …erve all visual interactions
    
    This is a big commit with a large potential for bugs. Think of this as a spike or POC for now. There'll be a lot to address. Some general TODOs:
    - [ ] Refactor shared code between empty-block-inserter and selected-block-tools
    - [ ] More explicitly pass the popover slot and content ref into the SelectedBlockTools
    - [ ] Shortcut/keystrokes for returning from the toolbar to where the cursor was before moving into the toolbar
    - [ ] Inline Tools either move to the top toolbar or get inserted into the main block toolbar (think image caption formatting tools)
    - [ ] Visual styles for the top toolbar
    jeryj committed Sep 6, 2023
    Configuration menu
    Copy the full SHA
    5d7bda7 View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    c593e05 View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    4e0c909 View commit details
    Browse the repository at this point in the history
  4. Configuration menu
    Copy the full SHA
    53df7d3 View commit details
    Browse the repository at this point in the history
  5. Configuration menu
    Copy the full SHA
    f0da0e8 View commit details
    Browse the repository at this point in the history
  6. Configuration menu
    Copy the full SHA
    9fc803c View commit details
    Browse the repository at this point in the history
  7. Remove unnecessary z-index

    jeryj committed Sep 6, 2023
    Configuration menu
    Copy the full SHA
    5257349 View commit details
    Browse the repository at this point in the history
  8. Configuration menu
    Copy the full SHA
    9bd48f9 View commit details
    Browse the repository at this point in the history
  9. Configuration menu
    Copy the full SHA
    7fc9bb1 View commit details
    Browse the repository at this point in the history
  10. Configuration menu
    Copy the full SHA
    96b184b View commit details
    Browse the repository at this point in the history
  11. Configuration menu
    Copy the full SHA
    fd31f65 View commit details
    Browse the repository at this point in the history
  12. Register global shortcut for moving focus back to the last focused bl…

    …ock.
    
    Still more todo. This shouldn't go to the last focused block but the last _selection_ which will be a bit more work. Probably best to refactor how it works within use-tab-nav. Maybe turn it into a hook or something that can have centralized logic.
    jeryj committed Sep 6, 2023
    Configuration menu
    Copy the full SHA
    c3c76b9 View commit details
    Browse the repository at this point in the history
  13. Start on escape to return to block selection

    It's not working for buttons that contain dropdowns, such as the align or options buttons. The dropdown toggle is intercepting the escape keypress and not letting it bubble.
    jeryj committed Sep 6, 2023
    Configuration menu
    Copy the full SHA
    e747b88 View commit details
    Browse the repository at this point in the history
  14. Configuration menu
    Copy the full SHA
    3b696bc View commit details
    Browse the repository at this point in the history
  15. Configuration menu
    Copy the full SHA
    732fabb View commit details
    Browse the repository at this point in the history
  16. Handle Escape on Toolbar Option 1: Add event listeners to all childre…

    …n via onChildrenKeyDown passed to NavigableToolbar
    
    Note: Due to the use of portals in the toolbar slots, consistently handling event listeners cleanly is difficult.
    
    This method adds event listeners to all children of a toolbar if a `onChildrenKeyDown` prop is passed to the <NavigableToolbar /> via a querySelectorAll( '[data-toolbar-item="true"]' ) on the NavigableToolbar ref, then applying an addEventListener to all the returned buttons.
    
    Pros: Event listener is applied and handled in one location.
    Cons: Feels yucky. Feels like crossing our fingers and hoping it works consistently, which it might.
    jeryj committed Sep 6, 2023
    Configuration menu
    Copy the full SHA
    9acb93d View commit details
    Browse the repository at this point in the history
  17. Handle Escape on Toolbar Option 2: Use addEventListener on the select…

    …edblock toolbar, attached to the NavigableToolbar
    
    This method requires forwarding a ref from the editor level down to the navigable toolbar so that the escape unselect shortcut can be blocked and the navigable toolbar event listener will still fire.
    
    Blocking the global escape event shouldn't be necessary, but we have a combination of a few things all combining to create a situation where:
    - Because the block toolbar uses createPortal to populate the block toolbar fills, we can't rely on the React event bubbling to hit the onKeyDown listener for the block toolbar
    - Since we can't use the React tree, we use the DOM tree which _should_ handle the event bubbling correctly from a `createPortal` element.
    - This bubbles via the React tree, which hits this `unselect` escape keypress before the block toolbar DOM event listener has access to it.
    jeryj committed Sep 6, 2023
    Configuration menu
    Copy the full SHA
    b5f5248 View commit details
    Browse the repository at this point in the history
  18. Handle Escape on Toolbar Option 3: Remove bubblesVirtually from block…

    …-controls slot
    
    If remove the bubblesVirtually from the block-controls slot, then the event will bubble in the DOM as normal (instead of the React Tree only since bubblesVirtually uses createPortal for the fills). This means we could handle the event without any forwardRefs as we don't need to block the escape shortcut keypress event from the React Tree.
    
    However, we assume bubblesVirtually was made for a reason. So, we're unsure if something would break.
    jeryj committed Sep 6, 2023
    Configuration menu
    Copy the full SHA
    401cf4c View commit details
    Browse the repository at this point in the history
  19. Configuration menu
    Copy the full SHA
    25a79f2 View commit details
    Browse the repository at this point in the history

Commits on Sep 11, 2023

  1. Configuration menu
    Copy the full SHA
    384e717 View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    399afc6 View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    051fd41 View commit details
    Browse the repository at this point in the history
  4. Configuration menu
    Copy the full SHA
    44851dd View commit details
    Browse the repository at this point in the history

Commits on Sep 12, 2023

  1. Configuration menu
    Copy the full SHA
    5fc387c View commit details
    Browse the repository at this point in the history
  2. Add focusEditorOnEscape to NavigableToolbar to return focus to editor…

    … by default on escape keypress
    
    Not committed to this, but it does work since we've removed bubblesVirtually from the toolbar slots and the inline rich editor doesn't use slots for its tools. I like that this brings this behavior by default to the toolbars, but unsure about if it's a good idea overall.
    jeryj committed Sep 12, 2023
    Configuration menu
    Copy the full SHA
    a9b0710 View commit details
    Browse the repository at this point in the history