fix terminal toolbar reset menu visibility #171361
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
HesamSoleymani
force-pushed
the
fix-terminal-toolbar-reset-menu-hide
branch
from
00fe169 to
9588cf5
Compare
Ramin-RX7
approved these changes
Feb 8, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #171187
I'm solving this issue as a good-first-issue.
I think the ideal case would be considering the default hiddenness of menu items and then not showing the "reset menu" option in that default state. But currently, it doesn't show up the reset menu option when all items are visible (which is not the default state anymore after adding options like "Run active file" which are hidden by default).
Unfortunately, I couldn't find a way to access the
isHiddenByDefaultproperty in toolbar.ts (where showing or hiding the reset menu option is handled). I think It would be nice if we also have that property in theIActioninterface so that we can easily detect the default state and then decide whether to show or not show the reset menu option.For now, the only change in this pr is always showing the reset menu option which I think is at least better than the current state.