TINY-9337: Remove focusing after toggling the toolbar

[yacodes]

Related Ticket: TINY-9337

Description of Changes:

• Removed focusing after toggling the toolbar in FloatingToolbarButton.

The focusing was added in the initial FloatingToolbarButton commit in 2019. I am not sure what are the consequences of removing it, but it definitely conflicts with skip_focus argument in execCommend.

Other solutions would be to refactor the execCommand in order to pass down skip_focus argument to all commands, but that feels like a major public API change.

Pre-checks:

• Changelog entry added
• Tests have been added (if applicable)
• Branch prefixed with feature/, hotfix/ or spike/

Review:

• Milestone set
• Docs ticket created (if applicable)

GitHub issues (if applicable):

[ephox-mogran]

You need to be really careful when removing focus calls. Remember, that the editor needs to be completely keyboard accessible. If you remove this focus, then a keyboard user can't start navigating inside the floating toolbar more section once they open it with an <enter> key.

Similarly, you need to check that pressing <escape> inside the floating toolbar section will close the section and focus the triggering button. It seems this is still working, because it is probably being done by something else as well ... but would that mean that when closing skip_focus is being ignored?

You'll also need to update the test: modules/alloy/src/test/ts/browser/ui/button/FloatingToolbarButtonTest.ts. It looks like it should probably have another test to explicitly check focus has transferred into the floating section after being opened (by the keyboard or mouse), to more clearly identify that this is a requirement.

-- In terms of how to fix this

Ultimately, we might need a different code path for a "silent" trigger of the menu, so that it doesn't impact when the user actually triggers it via a keyboard or mouse, but that's just a thought.

[spocke]

I'm with @ephox-mogran this probably need to be that harder path of supporting the skip_focus parameter for the command. I think we can string that though by just passing in the extra args at the end of the function that you register for a command. Currently when you register a command you provide a function with 2 parameters if you can have 3 and the new one is option so basically (ui, value?, args?). But not sure how to handle the not focusing part how to string that though all of silver.

[yacodes]

I was thinking about an additional parameter too. We do have a couple of callbacks which react to grabFocus key argument. Maybe we should add the similar behaviour here as well.

[yacodes]

@ephox-mogran @spocke Updated the PR with the additional skipFocus: true option for the ToggleToolbarDrawer command. It took a bit of refactoring to pass it all the way down to the Alloy components, making the changes a bit big.

[TheSpyder]

I think it's ok? If morgan is happy I'm happy (so I'm not approving, since I won't be around to follow up for 3 weeks)

[ephox-mogran]

Some minor things, but I'll approve it.

The singleton will bite us, though :P