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

Extensions use notifications too much to notify the user to reload #121859

Closed
isidorn opened this issue Apr 21, 2021 · 11 comments
Closed

Extensions use notifications too much to notify the user to reload #121859

isidorn opened this issue Apr 21, 2021 · 11 comments
Assignees
Labels
debt Code quality issues extensions Issues concerning extensions *out-of-scope Posted issue is not in scope of VS Code polish Cleanup and polish issue
Milestone

Comments

@isidorn
Copy link
Contributor

isidorn commented Apr 21, 2021

Refs: #119463

I noticed that in the extensions land we often use the notifications to let the user know that he needs to reload in order for the extension to be fully instaled / uninstalled, while in some cases I guess that makes sense overall I think:

  • We shuold not do this when the user uninstalled an extensions
  • Here since the user was already asked via a model dialog. So if he says yes, we should automtatically install and reload, and not ask again via a notification
@sandy081
Copy link
Member

I think the problem here is that install is a long running operation and reload can happen after a moment when user might be doing something. This might be annoying if we just reload then?

@sandy081 sandy081 added the info-needed Issue requires more information from poster label Apr 22, 2021
@isidorn
Copy link
Contributor Author

isidorn commented Apr 22, 2021

If it was user initiated I think it makes sense to experiment with automatic reloading (if extension requires it).
Maybe we could do this change start of a milestone on insiders so we get a feeling for how it would behave. If we do not try we will not know if it will be annoying :)
Of course the button would need to be updated from something like "Install" to "Install and Reload"

Also reload is pretty fast so that is why I am optimistic in this flow.

@sandy081 sandy081 removed the info-needed Issue requires more information from poster label Apr 22, 2021
@sandy081
Copy link
Member

Fine with me. But cc-ing @joaomoreno as he is the primary owner for url handling

@joaomoreno
Copy link
Member

No sure what this has to do with url handling? This prompt is necessary because it is destructive.

@isidorn
Copy link
Contributor Author

isidorn commented Apr 22, 2021

I created this issue more to tackle many cases where we install extensions and offer a notification for the reload.

My idea was if the extension install is user triggered could we compress this into one action "Install + Reload", mostly in that time period between the two there is nothing that the user can do that can be destructed.

But in general I like our update flow. Update gets installed, badge in gear -> reload to update.
Why can't we have a similar flow for extensions. Extensions gets installed, badge in extension viewlet -> some action in there to reload.

@joaomoreno joaomoreno removed their assignment Apr 22, 2021
@sandy081
Copy link
Member

Can we be specific here about what issue we are discussing? Are we talking about general extension installation or this specific case where user tries to open an URL that is handled by an extension that is not installed?

In general extension installation, we do not show reload because we enable the extension automatically.

In this case (extension url handling) - URL handler installs the extension and it is up to it to reload or not or handle the url immediately.

My recommendation would be not to generalize this problem and go case by case as every feature requiring installing extension has its own requirements. So in this case, I would request @joaomoreno to decide what he would like to do after installing the extension.

BTW I am all in for showing a badge in the extensions view for those extensions requiring reload - I already have this feature request. But is this going to fix and improve this scenario ?

@isidorn
Copy link
Contributor Author

isidorn commented Apr 23, 2021

@sandy081 going case by case makes sense, sorry for not making this more explicit. So here's the list of usages where we offer to reload:

My point is that looking at the code we seem to be doing this a lot and introducing a general approach to this problem might help.
@sandy081 you of course know each usage much better than me, I have simply glanced at this and I do think that the Extensions badge can improve a lot of these scenarios. Or one install + reload button.

@sandy081
Copy link
Member

@isidorn I updated my comments in the above list and asked @joaomoreno @lszomoru for their comments in their scenarios

@isidorn
Copy link
Contributor Author

isidorn commented Apr 27, 2021

@sandy081 great, thanks a lot!
Can we maybe assign this to May or June so we do not loose track of this?

@joaomoreno
Copy link
Member

I think keeping the URL handling notification makes sense.

@sandy081 sandy081 added this to the May 2021 milestone Apr 28, 2021
@sandy081 sandy081 added debt Code quality issues extensions Issues concerning extensions polish Cleanup and polish issue labels Apr 28, 2021
@sandy081 sandy081 modified the milestones: May 2021, Backlog, On Deck Jun 3, 2021
@sandy081 sandy081 modified the milestones: On Deck, Backlog Nov 5, 2021
@isidorn isidorn added the *out-of-scope Posted issue is not in scope of VS Code label Dec 6, 2022
@VSCodeTriageBot
Copy link
Collaborator

We closed this issue because we don't plan to address it in the foreseeable future. If you disagree and feel that this issue is crucial: we are happy to listen and to reconsider.

If you wonder what we are up to, please see our roadmap and issue reporting guidelines.

Thanks for your understanding, and happy coding!

@VSCodeTriageBot VSCodeTriageBot closed this as not planned Won't fix, can't repro, duplicate, stale Dec 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
debt Code quality issues extensions Issues concerning extensions *out-of-scope Posted issue is not in scope of VS Code polish Cleanup and polish issue
Projects
None yet
Development

No branches or pull requests

4 participants