Close active menubar dropdown when the command palette is launched

[garethwhittaker]

What it does

Fixes: #6847
The command palette currently overlaps an active menubar dropdown when launched via one of its keybindings.

Apart from this potentially being a less than ideal UX, toggles triggered through the command palette such as View: Toggle Minimap would not have their updated state reflected in the overlapped dropdown.
Closing the menubar dropdown on command palette launch avoids this complication.

** Update (12/02/20)
An active menubar dropdown should now be closed when launching any quick palette.

How to test

Open a menubar dropdown and then launch a quick palette via one a keybindings.
The menubar dropdown should now be closed rather than overlapped by the quick palette.
Keyboard focus should be on the quick palette input.

Review checklist

• as an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

• as a reviewer, I agree to behave in accordance with the review guidelines

[vince-fugnitto]

Thank you for your contribution @garethwhittaker!

I verified the changes when using the command palette and it successfully dismissed any opened menu. I'm wondering however if the change should be generalized to dismiss the menu whenever any quick-open is called, for instance when performing a file-search (ctrlcmd+p).

For reference, VS Code also dismisses any opened menu in this case:

[akosyakov]

@vince-fugnitto yes, it does not make sense to dismiss it only for quick command, but for the quick palette in general.

[garethwhittaker]

Thanks for the feedback.

I've made an update to the pull request extending this logic to the launch of other quick palettes that could also overlay an active menu dropdown.

[akosyakov]

code-wise it looks good to me, it would help if someone verifies that it works in browser and does not break electron

[vince-fugnitto]

I verified the changes in the browser and it works correctly.
I also tested for any regressions in the electron example and could not find any.

[RomanNikitenko]

@garethwhittaker
I tested it for browser - it works as described in the PR description, I don't see any regressions.

@vince-fugnitto

For reference, VS Code also dismisses any opened menu in this case

I tried the behavior for VS Code and can see that VS Code prevents to open the command palette if I have a menubar dropdown.
So when I have a open menubar dropdown and I'm trying to use keybindings to open the command palette - nothing is happening.

I don't mind the current PR changes - it works good for me!
Just to be aware - because from your comment I see that we have the different behavior for VS Code.
Sorry, if I misunderstood something.

[vince-fugnitto]

@RomanNikitenko

So when I have a open menubar dropdown and I'm trying to use keybindings to open the command palette - nothing is happening.

That is the behavior I noticed from VS Code (macOS). If a menu entry is open, triggering the command palette for instance, dismisses the menu.

[RomanNikitenko]

@vince-fugnitto
It's interesting, I updated the version of VS Code, but still can see the same behavior

Also I did not find the preference which could manage the behavior.
Maybe we have the difference because I tried on Ubuntu, but you use macOS.

Anyway I don't mind the current PR behavior, as I mentioned above.
thank you for the answer!

[akosyakov]

Different operating systems? Different ways how Electron integrates menu support for them?

[RomanNikitenko]

Different operating systems? Different ways how Electron integrates menu support for them?

Looks like it's really related to operating systems.
I can see the different behavior for 2 machines on Ubuntu vs 2 machines on macOS in our office