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

monaco: fix quick-command separator #9783

Merged
merged 1 commit into from
Jul 29, 2021
Merged

Conversation

vince-fugnitto
Copy link
Member

What it does

The following commit fixes the other commands separator which should be present only when there are recently used commands.

quick-command.mp4

How to test

  1. start the application.
  2. trigger the quick-command palette (F1).
  3. ensure that the list of recently used commands are separated from other commands.
  4. execute commands to add them to the recently used list.

Review checklist

Reminder for reviewers

Signed-off-by: vince-fugnitto vincent.fugnitto@ericsson.com

The following commit fixes the `other commands` separator which should
be present only when there are recently used commands.

Signed-off-by: vince-fugnitto <vincent.fugnitto@ericsson.com>
@vince-fugnitto vince-fugnitto added the monaco issues related to monaco label Jul 27, 2021
@vince-fugnitto vince-fugnitto self-assigned this Jul 27, 2021
Copy link
Contributor

@alvsan09 alvsan09 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have tested the following scenarios and work as expected. 👍

  1. Exercise new commands and make sure a separator is included before 'otherItems'
  2. Clear command history and make sure no separator is included

The code looks good to me as well !

Symbolic approval as I am not yet a committer.

Copy link
Member

@msujew msujew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can confirm that the separator only appears if there are recently used commands in the list. This aligns the behavior with vscode 👍

Copy link
Contributor

@colin-grant-work colin-grant-work left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes make sense, and now the other commands label only appears if there are recent items to contrast them with.

@vince-fugnitto vince-fugnitto merged commit 86995b7 into master Jul 29, 2021
@vince-fugnitto vince-fugnitto deleted the vf/quick-command branch July 29, 2021 14:23
@github-actions github-actions bot added this to the 1.16.0 milestone Jul 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
monaco issues related to monaco
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants