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 activeItem handling #11438

Merged
merged 1 commit into from
Jul 20, 2022
Merged

monaco: fix activeItem handling #11438

merged 1 commit into from
Jul 20, 2022

Conversation

vince-fugnitto
Copy link
Member

@vince-fugnitto vince-fugnitto commented Jul 18, 2022

What it does

Fixes #10454.
Fixes #11436.

The pull-request fixes an issue where the activeItem was not being respected when calling the showQuickPick API.
The activeItem is now respected and the item will be defaulted in the list and highlighted. The issue was that we were setting the activeItem before the items list had been set which resulted in nothing being set.

How to test

  1. confirm that Preferences: File Icon Theme will default to and highlight the active file icon theme when the menu is opened
  2. confirm that Preferences: Color Theme will default to and highlight the active color theme when the menu is opened
  3. confirm for a given extension that Install Another Version... will default to and highlight the current version
  4. confirm that Configure Display Language highlights the current language (regression test)
  5. confirm that Cannot read properties of undefined (reading 'id') when Preferences: Open Theme is triggered from a keybinding #11436 is fixed.

Review checklist

Reminder for reviewers

Signed-off-by: vince-fugnitto [email protected]

@vince-fugnitto vince-fugnitto added the monaco issues related to monaco label Jul 18, 2022
The commit fixes an issue where the `activeItem` was not being respected when calling the `showQuickPick` API.
The `activeItem` is now respected and the item will be defaulted in the list and highlighted.

Signed-off-by: vince-fugnitto <[email protected]>
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.

Behavior as described, and the cleanup of the 'cannot read ID of undefined' fix is a nice bonus 👍

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.

The changes also look good to me 👍

@vince-fugnitto vince-fugnitto merged commit 0e51680 into master Jul 20, 2022
@vince-fugnitto vince-fugnitto deleted the vf/fix-activeItem branch July 20, 2022 17:31
@github-actions github-actions bot added this to the 1.28.0 milestone Jul 20, 2022
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
3 participants