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

Open File... selection is first file rather than last / In quick input move active item to the very first item in the list rather than the very last #202406

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

CrafterKolyan
Copy link
Contributor

This is inspired by #202367. This doesn't fix it completely as no fuzzy search is working, though at least it makes it easier to reveal stuff in all lists inside VS Code as now if the item was not visible it will appear at the top rather than at the very bottom.

See video for the new behaviour:
https://github.com/microsoft/vscode/assets/9883873/eca0a3e3-2e1c-4339-b8da-72d45816e96f

@CrafterKolyan
Copy link
Contributor Author

@microsoft-github-policy-service agree

@CrafterKolyan
Copy link
Contributor Author

@Yoyokrazy Hi Michael. What is the policy for contributions from people outside of Microsoft? Does it make sense for me to believe that this will be reviewed any time soon or should I assume that the review process will take at least a month?

Right now I have two possible paths:

  1. Wait for this pull request to be reviewed and released in the VS Code
  2. Create my own VS Code extension which fixes the problems with Open File... dialog

I'm fine with both, though would prefer the first one as you already have a lot of code written here, but if you understand that almost surely this feature will take a very long time to review then please tell me so I start working on the VS Code extension instead without having false expectations

@Yoyokrazy
Copy link
Contributor

Yoyokrazy commented Jan 18, 2024

@CrafterKolyan Sorry about the delay, I've gone ahead and assigned a more relevant dev to this and the linked issue. cc @benibenj

@Yoyokrazy Yoyokrazy assigned benibenj and unassigned Yoyokrazy Jan 18, 2024
@benibenj benibenj assigned TylerLeonhardt and unassigned benibenj Jan 19, 2024
@@ -1865,7 +1865,7 @@ export class List<T> implements ISpliceable<T>, IDisposable {
return this.getFocus().map(i => this.view.element(i));
}

reveal(index: number, relativeTop?: number, paddingTop: number = 0): void {
reveal(index: number, relativeTop?: number, paddingTop: number = 0, applyRelativeTopOnlyIfNotVisible: boolean = false): void {
Copy link
Contributor

Choose a reason for hiding this comment

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

There should not be a change to the list widget. The consumer of the list should be able to adapt the arguments depending on if an item is visible or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for review. Agree the code now looks much nicer. Fixed.

@TylerLeonhardt TylerLeonhardt requested review from alexr00 and TylerLeonhardt and removed request for TylerLeonhardt and alexr00 January 24, 2024 04:48
@CrafterKolyan
Copy link
Contributor Author

@TylerLeonhardt Hi Tyler, any chance to review it during this week?

@TylerLeonhardt
Copy link
Member

Hi @CrafterKolyan, I'm having a hard time understanding the actual change in behavior here. Can you help me understand a bit more? Perhaps 2 gifs that show the same thing with the difference?

@CrafterKolyan
Copy link
Contributor Author

Hi @TylerLeonhardt, see the video below
https://github.com/microsoft/vscode/assets/9883873/1167f2c6-e696-46ba-b17e-c69b3ed46c03

On the left you see the way Open File... currently works in VS Code
On the right you see the behaviour with this change

Currently it's harder to discover there's folder userData while typing user, because you will only see userActivity (and it will be the last option in the list)

@CrafterKolyan
Copy link
Contributor Author

@TylerLeonhardt any updates on the review? I'm not sure if my video helped you to understand the difference. Also I'm not sure if you asked me to do gifs because you need specifically gifs (because e.g. you can't watch video due to too restricting policy of Microsoft or something).

@CrafterKolyan CrafterKolyan force-pushed the simple-dialog-select-top-instead-of-last branch from 1cc507a to f7f0921 Compare February 10, 2024 19:13
@TylerLeonhardt
Copy link
Member

Hi @CrafterKolyan haven't forgotten about this but I'm a bit busy with some other things and a change like this I wanna take time and do some manual tests. The video you supplied is helpful! Thanks!

@TylerLeonhardt TylerLeonhardt added this to the Backlog milestone Feb 13, 2024
@vivodi
Copy link

vivodi commented Dec 29, 2024

Is this still relevant?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants