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

Productise or remove explorer experimental custom hover #205061

Closed
Tracked by #212907
bpasero opened this issue Feb 13, 2024 · 9 comments · Fixed by #212865
Closed
Tracked by #212907

Productise or remove explorer experimental custom hover #205061

bpasero opened this issue Feb 13, 2024 · 9 comments · Fixed by #212865
Assignees
Labels
debt Code quality issues file-explorer Explorer widget issues insiders-released Patch has been released in VS Code Insiders perf-bloat
Milestone

Comments

@bpasero
Copy link
Member

bpasero commented Feb 13, 2024

renderTemplate(container: HTMLElement): IFileTemplateData {
const templateDisposables = new DisposableStore();
const experimentalHover = this.configurationService.getValue<boolean>('explorer.experimental.hover');
const label = templateDisposables.add(this.labels.create(container, { supportHighlights: true, hoverDelegate: experimentalHover ? this.hoverDelegate : undefined }));
templateDisposables.add(label.onDidRender(() => {
try {
if (templateData.currentContext) {
this.updateWidth(templateData.currentContext);
}
} catch (e) {
// noop since the element might no longer be in the tree, no update of width necessary
}
}));
const contribs = explorerFileContribRegistry.create(this.instantiationService, container, templateDisposables);
const templateData: IFileTemplateData = { templateDisposables, elementDisposables: templateDisposables.add(new DisposableStore()), label, container, contribs };
return templateData;
}

@bpasero bpasero added debt Code quality issues file-explorer Explorer widget issues labels Feb 13, 2024
@benibenj
Copy link
Contributor

I'll make sure to adapt this when I finish the hover work for lists

@bpasero
Copy link
Member Author

bpasero commented Apr 29, 2024

@lramos15 is the idea of the setting to be able to disable the hover in the explorer (#173236)? Why did it never become a normal option then but experimental?

@lramos15
Copy link
Member

Redmond agreed that disabling the hover completely was a bad idea since it would open the door up for people requesting settings for disabling the hover in many different places. Instead we aimed to solve the main problem I heard about the explorer hover which is that it does not include useful information and can actually obstruct the information the explorer displays. For this, we aimed to try to implement a hover similar to what windows does but it ended up causing too many regressions

@bpasero
Copy link
Member Author

bpasero commented Apr 29, 2024

@lramos15 but people that are annoyed by the hover can configure a high hover delay (I actually do that) and then this should be OK no?

@lramos15
Copy link
Member

That messes with all workbench hovers though

@benibenj
Copy link
Contributor

@lramos15 do we have users using the experimental custom hover? I also gave it a try to have it similar to the windows explorer but it is not possible to do this well. So personally I think we can remove the experimental explorer hover.

@benibenj
Copy link
Contributor

Regarding #173236: I agree with @lramos15. Changing the hover delay should not be the solution as it affects all hovers in the workbench. Also adding a setting for disabling the hover in the explorer might have the danger of users wanting to support it in all other places individually which would not be great, but we still need to find a better solution here so having a setting for just the explorer might have to be a solution. Another option could be to show the hover to the side of the element making sure it does not cover any other elements

@lramos15
Copy link
Member

@benibenj I think we can just remove the explorer hover code. It is very broken and not something we support, regardless of the users. I don't know anyone who is using it as I have not heard any discussion about it for quite some time.

@bpasero
Copy link
Member Author

bpasero commented May 16, 2024

Then lets do that.

@VSCodeTriageBot VSCodeTriageBot added the unreleased Patch has not yet been released in VS Code Insiders label May 16, 2024
@VSCodeTriageBot VSCodeTriageBot added insiders-released Patch has been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels May 17, 2024
@microsoft microsoft locked and limited conversation to collaborators Jun 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debt Code quality issues file-explorer Explorer widget issues insiders-released Patch has been released in VS Code Insiders perf-bloat
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants