Autocomplete for extension search @-operators #53915
Conversation
| this.disposables.push(addStandardDisposableListener(this.searchBox, EventType.BLUR, () => removeClass(this.searchBox, 'synthetic-focus'))); | ||
| this.monacoStyleContainer = append(header, $('.monaco-container')); | ||
| this.searchBox = this.instantiationService.createInstance(CodeEditorWidget, this.monacoStyleContainer, SEARCH_INPUT_OPTIONS, {}); | ||
| this.placeholderText = append(this.monacoStyleContainer, $('.search-placeholder', null, 'Search Extensions in Marketplace')); |
There was a problem hiding this comment.
The text here should be localized
| @@ -468,12 +510,17 @@ export class ExtensionsViewlet extends ViewContainerViewlet implements IExtensio | |||
| return this.panels.reduce((count, view) => (<ExtensionsListView>view).count() + count, 0); | |||
| } | |||
|
|
|||
| private onEscape(): void { | |||
| this.search(''); | |||
| private autoComplete(query: string, position: number): { fullText: string, additionalText: string, overwrite: number }[] { | |||
There was a problem hiding this comment.
The additionalText in the returned object doesnt seem to be used anywhere
| label: item.fullText, | ||
| insertText: item.fullText, | ||
| overwriteBefore: item.overwrite, | ||
| sortText: item.fullText.indexOf(':') === -1 ? 'a' : item.fullText.indexOf('sort') !== -1 ? 'b' : 'c', // things with no colon; sorts; everything else |
There was a problem hiding this comment.
Is the idea for this sorting to show the pre-defined views first, followed by the most commonly used "@sort", and then the rest?
This makes tag and ext go to the very bottom. We can probably bring them up.
How about this:
There was a problem hiding this comment.
It doesn't make sense to me to have "sort:" and "category:" in the list, as a user would never want to select them. We could just change the sort algorithm to put "ext:" and "tag:" before "category:*"?
| const onKeyDownForList = onKeyDown.filter(() => this.count() > 0); | ||
| onKeyDownForList.filter(e => e.keyCode === KeyCode.Enter).on(this.onEnter, this, this.disposables); | ||
| const onKeyDownMonaco = chain(this.searchBox.onKeyDown); | ||
| onKeyDownMonaco.filter(e => e.keyCode === KeyCode.Tab).on(e => e.stopPropagation(), this, this.disposables); |
There was a problem hiding this comment.
Pressing tab when the suggest widget is open should result in the selected suggestion being accepted. But this doesnt happen due to the above.
There was a problem hiding this comment.
For this and below, I don't know of a good way to enable keys based on the suggest widget being open. the repl does something with contextkeys that's moderately complicated and doesn't actually work all that well.
There was a problem hiding this comment.
I'll look into what we do in the core editor
There was a problem hiding this comment.
So core editor doesn't actually help much here I think, because in core editor they don't need to disable tabbing inside the editor. Unfortunately I cant find a way to get a trigger when the dropdown closes (the dropdown can close on click in addition to tab/esc/enter; we can listen to the keys, but not the click)
|
|
||
| this.extensionsBox = append(this.root, $('.extensions')); | ||
|
|
||
| const onKeyDown = chain(domEvent(this.searchBox, 'keydown')) | ||
| .map(e => new StandardKeyboardEvent(e)); | ||
| onKeyDown.filter(e => e.keyCode === KeyCode.Escape).on(this.onEscape, this, this.disposables); |
There was a problem hiding this comment.
why are removing the binding of esc key to clearing of the text?
There was a problem hiding this comment.
Both because of the above regarding the escape needing to operate on the suggest widget, but also more generally is "esc" to clear input a common thing? We use it to get rid of modals, which technically also clears their input, but besides that we don't use it anywhere else in the program.
There was a problem hiding this comment.
Looking around on Win10 and MacOS it turns out there are a fair number of places where Esc clears input (I've personally never known about that until now). But we still don't do in in Code, and browsers inputs don't do it by default
There was a problem hiding this comment.
Oh, we do it in settings search and keybindings. But still nowhere in the viewlets. I don't know what the best move is here.
There was a problem hiding this comment.
As discussed, lets not try to add this back. esc when suggest widget is open should remove the widget, otherwise esc need not do anything
| } | ||
|
|
||
| private onEnter(): void { | ||
| (<ExtensionsListView>this.panels[0]).select(); | ||
| (<ExtensionsListView>this.panels[0]).focus(); |
There was a problem hiding this comment.
On enter, I see that we reach this line of code. But visually I dont see any focus changing. So what does this do?
There was a problem hiding this comment.
Select also didn't do anything... this is dependent on #53616. I could bring those changes into this PR and close that one?
There was a problem hiding this comment.
as discussed, lets not try to do anything on Enter or Escape and focus on getting the tab to work
| vertical: 'hidden' | ||
| }, | ||
| ariaLabel: 'Search Extensions in Marketplace', |
There was a problem hiding this comment.
This should be localized as well.
| let SEARCH_INPUT_OPTIONS: IEditorOptions = | ||
| { | ||
| fontSize: 13, |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
JacksonKearl
force-pushed
the
autocomplete
branch
from
0032efa to
4fb6ff5
Compare
|
Tab handling is fixed dependent on #54672. |
|
fyi @sandy081 |
Closes #46333
This PR replaces the simple input box for extension search in the extension viewlet with a monaco editor with auto-complete support.
The auto-completion support is for pre-defined filters