Skip to content

preferences: Fix search term on open via menu#9932

Merged
colin-grant-work merged 1 commit intomasterfrom
ms/fix-preferences-menu-action
Aug 20, 2021
Merged

preferences: Fix search term on open via menu#9932
colin-grant-work merged 1 commit intomasterfrom
ms/fix-preferences-menu-action

Conversation

@msujew
Copy link
Member

@msujew msujew commented Aug 19, 2021

What it does

Closes #9931

Fixes the issue by simply checking for the string type before passing the argument into the search term.

How to test

  1. Open the preferences menu via the settings menu in the bottom left.
  2. Observe an empty search bar

Review checklist

Reminder for reviewers

@msujew msujew added the preferences issues related to preferences label Aug 19, 2021
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

@msujew I confirmed that it fixes the issue with the preferences, however I believe it introduces a regression in the following use-case:

pull-request:

context-menu-regression.mp4

master:

menu-master.mp4

@colin-grant-work
Copy link
Contributor

@msujew I confirmed that it fixes the issue with the preferences, however I believe it introduces a regression in the following use-case:

Very timely: there's the legitimate use case for it (maybe - I'm not sure that's how the menu should work...).

@msujew
Copy link
Member Author

msujew commented Aug 19, 2021

however I believe it introduces a regression

Ah, as I thought we rely somewhere on that undocumented behavior. :/

Alright, I'll look for a more localized fix for this issue.

@msujew msujew force-pushed the ms/fix-preferences-menu-action branch from ae01127 to c1cef27 Compare August 19, 2021 14:45
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.

This solves the problem nicely, and definitely looks like a safer approach. 👍

Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

I confirmed that the change fixes the issue on master 👍
It is also self-contained and does not cause any regressions.

@colin-grant-work colin-grant-work merged commit 0bc2375 into master Aug 20, 2021
@github-actions github-actions bot added this to the 1.17.0 milestone Aug 20, 2021
@vince-fugnitto vince-fugnitto deleted the ms/fix-preferences-menu-action branch August 20, 2021 03:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

preferences issues related to preferences

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Opening the preferences menu via menu automatically sets a search term

3 participants