Disable tooltips for Quick Input to prevent obscuring dropdown result (fix #285927) #286181
Conversation
📬 CODENOTIFYThe following users are being notified based on files changed in this PR: @bpaseroMatched files:
|
| input.ariaAutoComplete = 'list'; | ||
|
|
||
| // Hide tooltip when user starts typing to prevent obscuring dropdown results | ||
| this._register(dom.addDisposableListener(input, dom.EventType.KEY_DOWN, () => { |
There was a problem hiding this comment.
For clean code, this should be handled by the input that Quick Pick takes advantage of and be a configuration option on that object upon creation.
There was a problem hiding this comment.
Thanks for your review. I moved the logic to InputBox and added a boolean to the configuration called hideHoverOnKeyDown
| this.onfocus(this.input, () => this.onFocus()); | ||
|
|
||
| if (this.options.hideHoverOnKeyDown) { | ||
| this._register(dom.addDisposableListener(this.input, dom.EventType.KEY_DOWN, () => { |
There was a problem hiding this comment.
We already have an onValueChange function that runs when what is in the input changes. This feels like a good approximation without needing to add another event.
Additionally, rather than doing this, getBaseLayerHoverDelegate().hideHover(); we already have a this.hover.value which we could probably just dispose of instead.
There was a problem hiding this comment.
Yep you are totally right about using onValueChange. About getBaseLayerHoverDelegate().hideHover();, while testing, I tried setting this.hover = undefined (using this.hover.clear()) but it didn't really made the hover disappear from the UI
| readonly actionViewItemProvider?: IActionViewItemProvider; | ||
| readonly inputBoxStyles: IInputBoxStyles; | ||
| readonly history?: IHistory<string>; | ||
| readonly hideHoverOnKeyDown?: boolean; |
There was a problem hiding this comment.
last bit of feedback (I think). Let's change this to hideHoverOnValueChange since that is the actual event it is reacting to.
TylerLeonhardt
added
the
~copilot-video-please
vs-code-engineering
bot
removed
the
~copilot-video-please
|
🤖 Copilot has tried out this change! Members of the team can view the recording here or download the Playwright trace here (run) |
TylerLeonhardt
added
the
~copilot-video-please
vs-code-engineering
bot
removed
the
~copilot-video-please
|
🤖 Copilot has tried out this change! Members of the team can view the recording here or download the Playwright trace here (run) |
TylerLeonhardt
left a comment
TylerLeonhardt
left a comment
There was a problem hiding this comment.
Thanks! I think this is a good compromise
3 participants
Disable tooltips for Quick Input to prevent obscuring dropdown result (fix #285927)