Fix the focus management for buttons inside the grouped toolbar

[Dumluregn]

Suggested merge commit message (convention)

Fix (ui): Grouped toolbar no longer steals the focus from the editor after executing a button inside it. Closes #12178.

Fix (media-embed): Adding a media embed will now move the focus to editor editable. Closes #12186.

---

Additional information

1. The solution was tested and accepted by QA team (see comments in the issue - https://github.com/ckeditor/ckeditor5/issues/12178#issuecomment-1206434202).
2. Two tests started failing after this change (for esc and arrowleft keystrokes). They were assuming that the dropdown button will be focused when those keystrokes are executed, which isn't true in the given scenario - as the dropdown is never focused, it no longer gains focus out of nowhere. I've simply reversed that one assertion, but I'm not sure if it makes any sense now. Perhaps it would be better to remove this checks altogether.
3. As agreed, the fix was implemented in the render() method of the DropdownView class as it's a follow-up after #12162. However I'm not sure if that's the right way. The dropdown focus behavior is mostly managed through the helpers in utils.js file, like closeDropdownOnBlur(). Further splitting this logic between two files will make the whole flow harder to follow. I'd rather add a helper called focusButtonOnDropdownClose() which would add a separate listener:

function focusButtonOnDropdownClose( dropdownView ) {
    dropdownView.on( 'change:isOpen', () => {
        if ( !dropdownView.isOpen ) {
            if ( dropdownView.element.contains( document.activeElement ) ) {
                dropdownView.focus();
            }

            return;
        }
    }
}

and execute it in the addDefaultBehavior() function. Please reconsider this approach during the review. Functionally such a change should be transparent and not require the extensive re-testing.

[Dumluregn]

Hi @ckeditor/qa-team, since we've fixed #12229 please take a quick peek (no longer than 15 min I think) if the image insert focus issues mentioned in comments under #12178 are solved.

[mlewand]

@ckeditor/qa-team when tested please give R+ using GH's review system so that it's clearly visible for us 🙂 I believe that R+ from one of you is enough in this case.

[Acrophost]

Issue mentioned in https://github.com/ckeditor/ckeditor5/issues/12178#issuecomment-1205363543 is still not fully fixed.

• On balloon editor while adding image via url focus is still going missing (except desktop safari, where focus goes in a wrong place)
• In case of Safari@iOS (any editor) while inserting image via button the keyboard does not appear after the image is added

[Dumluregn]

Requested changes were applied. As for my own refactor proposal:

As agreed, the fix was implemented in the render() method of the DropdownView class as it's a follow-up after Ck/12125 dropdown focus #12162. However I'm not sure if that's the right way. The dropdown focus behavior is mostly managed through the helpers in utils.js file, like closeDropdownOnBlur(). Further splitting this logic between two files will make the whole flow harder to follow. I'd rather add a helper called focusButtonOnDropdownClose() which would add a separate listener:

I won't be able to cover it, perhaps someone from the team will handle it instead.

[mmotyczynska]

Requested changes were applied. As for my own refactor proposal:

As agreed, the fix was implemented in the render() method of the DropdownView class as it's a follow-up after Ck/12125 dropdown focus #12162. However I'm not sure if that's the right way. The dropdown focus behavior is mostly managed through the helpers in utils.js file, like closeDropdownOnBlur(). Further splitting this logic between two files will make the whole flow harder to follow. I'd rather add a helper called focusButtonOnDropdownClose() which would add a separate listener:

I won't be able to cover it, perhaps someone from the team will handle it instead.

Actually, it's not that simple, moving the code to utils.js causes some tests to fail. This is probably because this code is being executed on render and the code in addDefaultBehavior() is executing before render, so it may need some additional adjustments.
Besides, Olek is thinking about fixing this problem in a different way, so first let's wait and see how the code changes, and then we will decide if it's still worth moving it to utils.js.

[oleq]

I noticed that this PR does not address the issue of the "Show more items" toolbar staying visible. I forked this PR and addressed the issue there #12319 + some refactoring.

[Dumluregn]

Closing this PR in favor of https://github.com/ckeditor/ckeditor5/pull/12319.