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

Limit the number of prompts shown to the user #7700

Closed
karthiknadig opened this issue Oct 1, 2019 · 10 comments
Closed

Limit the number of prompts shown to the user #7700

karthiknadig opened this issue Oct 1, 2019 · 10 comments
Assignees
Labels
area-environments Features relating to handling interpreter environments feature-request Request for new features or functionality needs proposal Need to make some design decisions verified Verification succeeded
Milestone

Comments

@karthiknadig
Copy link
Member

The extension does not control the order OR the number of prompts shown simultaneously. Potentially linter, formatter, test settings, conda env, etc. can show up at once.

There is an added problem of calling blocking prompts in activate, which can cause extension to load to fail silently. #7633 (comment)

@karthiknadig karthiknadig added feature-request Request for new features or functionality feature-* triage-needed Needs assignment to the proper sub-team labels Oct 1, 2019
@ghost ghost removed the triage-needed Needs assignment to the proper sub-team label Oct 1, 2019
@karrtikr
Copy link

karrtikr commented Oct 1, 2019

There is an added problem of calling blocking prompts in activate, which can cause extension to load to fail silently. #7633 (comment)

Can we do anything about this @DonJayamanne ? Except for making sure we don't do this.

@DonJayamanne
Copy link

Can we do anything about this @DonJayamanne ? Except for making sure we don't do this.

Not, much, it's a bug in the code.
Have had this in the past.

All we can do is rely on our CI tests to fail, cuz extension will never activate, hence tests will timeout, then CI will go kaboom, at least that's what I expect.

@DonJayamanne
Copy link

Not, much, it's a bug in the code.

I take that back, a possible solution is creating a custom linter (we've done this in the past). Details here #7702

@karthiknadig
Copy link
Member Author

There is a way :) :
image

@DonJayamanne
Copy link

DonJayamanne commented Oct 1, 2019

Suggestions:

1. Linter

Create a custom linter (we've done this in the past). Details here #7702

2. Tests

Create function tests and ensure the extension isn't blocked by messages. Details here #7703

3. Design

  • I think we need to keep track of these messages that are displayed when extension loads
  • Over time this list will grow and reveal to us the number of these such messages, and it will be obvious that something needs to be done.

I.e. the more messages we have, it becomes obvious that our extension is spamming the user with too many messages and we'd be forced to re-think our approach.

At the end of the day it's a poor design that will need to be addressed.

E.g. we displayed prompts asking the user to upgrade their settings from one to the other. E.g. when we changed the names of some settings in the settings.json file and launch.json. Displaying a prompt makes no sense, as this must be done for extension to continue functioning correctly.
Ideally (as proposed) we could display a notification of such a message, letting the user know Hey we updated some of your settings, please see this link for further info. This could be a one time (generic) message per session. This way we don't display prompt for each change in each setting...

@karthiknadig
Copy link
Member Author

An example of the issue:
image

Another thing to consider when we show messages, like do they even work. If i have no interpreter selected, where is the install flake8 going to install it? It is unclear from the message what is going to happen.

@luabud luabud added needs proposal Need to make some design decisions and removed needs decision labels Oct 31, 2019
@luabud
Copy link
Member

luabud commented Dec 30, 2019

Additional feedback in #9281 from @isidorn:

Hi VS Code dev here 👋

Open a Python file, notice the Python Interpreeter notification poping up immediatly:
I find this behavior obtrusive because:

  • do we have to notify the user about the interpreter right away? Is there a smarter time to show this notification. Suggestion: only show this when the user cares about interpreting code (running a task, or starting debugging)
  • there is already a status bar item for this, can we make the status bar more clear as to what it does so the notification is not needed. Suggestion: use hover message to clarify what it does (hopefully this can be contributed)
  • the notification just keeps coming back until I click "Got It". Suggestion: if the user ever clicks on the status bar or changes the interpeter stop showing the notification

Idealy this notificaiton would not be shown at all, however if you disagree I believe that my suggestions from above can help with the bad user experience. Thanks

I would also suggest that you go through all your usages of notifications and try to minimize them, as they should usually be used as a last resort if there is no other way to convey a message to the user.

fyi @DonJayamanne @qubitron

Screenshot 2019-12-24 at 12 15 12

I started debugging for the first time and I got this notification:

I suggest to reduce the spam to users. Get users for surveys using twitter or just talke to them on GitHub do not use notifications.
If you have to use notifications please only ask after I have done at least 10 debugging seassons, this way my first experience is ruined.
Screenshot 2019-12-24 at 14 41 02

Also this should be shown when the user opens the test viewlet, not on startup
The test viewlet is empty and has enough space for spam like this

Screenshot 2019-12-24 at 16 13 54

What happens on startup for me (I do not have everything configured but still):
Screenshot 2019-12-24 at 21 19 02

@luabud luabud changed the title Limit the number of prompts shown to the user simultaneously Limit the number of prompts shown to the user Dec 30, 2019
@luabud luabud added area-environments Features relating to handling interpreter environments and removed feature-* labels May 13, 2020
@davido-bs
Copy link

The very first thing you see when installing the extension is the "Quick Start" that tells you how to change your interpreter, so I think the redundant "Tip" notification should just be removed entirely at this point.

I personally have a very strong aversion to any sort of these forced "clicking on a Got It! button" shenanigans.

@luabud
Copy link
Member

luabud commented Mar 24, 2021

#15764

@karrtikr karrtikr added this to the June 2022 milestone Jun 24, 2022
@karrtikr karrtikr self-assigned this Jun 24, 2022
@brettcannon brettcannon modified the milestones: June 2022, July 2022 Jun 30, 2022
@karrtikr
Copy link

Closing in favor of #19174

@karrtikr karrtikr added the verified Verification succeeded label Jul 26, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-environments Features relating to handling interpreter environments feature-request Request for new features or functionality needs proposal Need to make some design decisions verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

6 participants