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

Implement preview on highlighting quickpick in quick search #191259

Closed
andreamah opened this issue Aug 24, 2023 · 7 comments · Fixed by #202306
Closed

Implement preview on highlighting quickpick in quick search #191259

andreamah opened this issue Aug 24, 2023 · 7 comments · Fixed by #202306
Assignees
Labels
feature-request Request for new features or functionality insiders-released Patch has been released in VS Code Insiders on-release-notes Issue/pull request mentioned in release notes on-testplan search Search widget and operation issues

Comments

@andreamah
Copy link
Contributor

It would be nice to show a preview editor that changes in the background based on which quick search item you have open. Furthermore, it'd be nice to highlight the result like we do in the symbol editor.

Image

This might be sort of non-trivial to tackle, as existing pickers that change the background editor only deal with a single open editor. In our case, we would need to implement a preview editor for this.

@bpasero
Copy link
Member

bpasero commented Jan 3, 2024

@andreamah I think this is a duplicate of #8939

I think one challenge here is that while you show preview editors in the background, you do not accumulate or alter the editor state (there is some discussion around this in the referenced issue). For example, I would expect:

  • that cancelling quick pick restores editor UI as it was
  • that the editors that open in the background do not appear in the editor history (e.g. "Go Back")

We currently do not really have support for the latter.

That said, I find it nice how ST shows preview editors in the background without even opening a tab. Notice how the editor opens without a tab associated to it:

Recording 2024-01-03 at 15 50 43

//cc @TylerLeonhardt

@andreamah andreamah modified the milestones: On Deck, December / January 2024 Jan 3, 2024
@andreamah
Copy link
Contributor Author

andreamah commented Jan 3, 2024

Yeah, I can see how that's tricky. How I see this version is as follows (slightly summarized using #8939 (comment))

  1. If the editor with the match already exists as an open editor, use that editor to show the match. Moving to another match in another file or escape-ing the menu will restore that editor to its original line number.
  2. Otherwise, open up a preview editor at the match that can persist if the user presses enter.

In this, we don't need to worry about which line to open preview editors at, since we have a specific match to highlight.

As for the tab-less navigation, I agree that it looks nice, but does it align with anything else that we have? I think it would be slightly confusing to introduce new UX within this feature.

Is this main concern regarding the Go Back/Forward functionality? And if so, can't we just fix this by adding an option to openEditor that determines whether the open editor should contribute to the editor history?

As I see this right now, this whole feature involves extending AbstractEditorNavigationQuickAccessProvider to optionally support what I'm currently using from PickerQuickAccessProvider.

@bpasero
Copy link
Member

bpasero commented Jan 3, 2024

Just fyi, preview editors can be disabled (and many do so, including me). There is no way to enforce preview editors when they are disabled via settings currently.

As for the non-tab editor, I agree its probably out of place with our UI, but wanted to share it nevertheless.

As for the history, most of that is tracked in src/vs/workbench/services/history/browser/historyService.ts and is loosely coupled to the editor that opens.

How would we decide to open an editor in the background? If I type quickly in quick pick, many different results might appear as first result - selected. If we were to open these as preview editors it might look quite hectic (it actually is hectic in ST as well).

@andreamah
Copy link
Contributor Author

Just fyi, preview editors can be disabled (and many do so, including me). There is no way to enforce preview editors when they are disabled via settings currently.

Yeah, I think that things like the Replace Preview from search would just persist in that case also (but is a preview editor if the user allows it). I think that allowing the same behavior (preview editor if we can) is sufficient for this case also.

How would we decide to open an editor in the background? If I type quickly in quick pick, many different results might appear as first result - selected. If we were to open these as preview editors it might look quite hectic (it actually is hectic in ST as well).

I would think that proper debouncing would help. But I agree that we would often end up opening the first result and it would be annoying for that to persist. I guess there's no other way to have a 'temporary' editor other than with preview editors, right?

@TylerLeonhardt
Copy link
Member

I do wonder If the tooltip UX would be a good option here.
image

This could show a smaller version of the file to the right of the quick pick.

It could be debounced so it doesn't show up too fast.

seems like it could be less jarring than changing the editor in the background

@andreamah
Copy link
Contributor Author

I'm not too sure whether people would want a tooltip since:

  1. Users might want more context on the file.
  2. Tooltips usually take ~1 second to show up, which might be sort of slow for some users who are quickly going through results.

I think the tooltip is a good idea, though! Just maybe not for this particular use case?

@TylerLeonhardt
Copy link
Member

Just to be clear, I'm specifically talking about quickpickitem tooltips which can be triggered instantly with ctrl+space and can have rich content in them.

Think of it like the "more info" tooltip in the editor when you do ctrl+space of a completion item

@VSCodeTriageBot VSCodeTriageBot added unreleased Patch has not yet been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels Jan 18, 2024
@VSCodeTriageBot VSCodeTriageBot added the insiders-released Patch has been released in VS Code Insiders label Jan 19, 2024
@andreamah andreamah added the on-release-notes Issue/pull request mentioned in release notes label Jan 26, 2024
@aiday-mar aiday-mar added this to the December / January 2024 milestone Feb 6, 2024
@microsoft microsoft locked and limited conversation to collaborators Jun 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality insiders-released Patch has been released in VS Code Insiders on-release-notes Issue/pull request mentioned in release notes on-testplan search Search widget and operation issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants