-
Notifications
You must be signed in to change notification settings - Fork 30.2k
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
Dont update disabled extensions #58723
Conversation
@ramya-rao-a It's an easy one but not sure making it default might make other users happy. Not showing the truth about an extension especially when others are showing may not be intended. We might have mitigated this issue already by not showing them in the default view. If this is necessary for users this can go behind a flag in my opinion. What do you think? |
There are 3 UI locations related to extension updates
In my opinion the count on the badge should definitely not include the disabled extensions. This is a pro-active information we show and are calling the user to take an action. And the action on disabled extensions is not urgent. We could update the notification from With the I'll make the changes appropriately. |
@sandy081 Done. |
@ramya-rao-a I am still bit uncertain to show different update badge numbers depending on disabled extensions. Did not it confuse users if the two VS Code windows show two different badge numbers? If users really want that this feature I would go for opt in rather than giving default. |
I am hesitant to add a new setting for a feature that will be used only by a very small section of our user base. Updated the PR to update the hover text on the badge to show the same message as the This should be a good middle ground for both sides of the argument. |
@ramya-rao-a Sorry I think I buy your argument to have the badge showing only for enabled extensions. As you said it is prompting user to take some action and it should not when an extension is explicitly disabled. Please revert the last commit to go to previous change and push. Thanks. |
LGTM |
Reverted the last change. |
Fixes #22461
This PR makes changes to avoid any update related UI for disabled extensions
cc @sandy081