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

UX improvements for automated typings acquisition #15209

Closed
mjbvz opened this issue Nov 8, 2016 · 19 comments
Closed

UX improvements for automated typings acquisition #15209

mjbvz opened this issue Nov 8, 2016 · 19 comments
Assignees
Labels
plan-item VS Code - planned item for upcoming verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@mjbvz
Copy link
Collaborator

mjbvz commented Nov 8, 2016

Adopt new status code API to improve automatic typing acquisition UX

@mjbvz
Copy link
Collaborator Author

mjbvz commented Nov 28, 2016

Opened microsoft/TypeScript#12540 to track the addition of the notifications we need from TS to support this

@mjbvz
Copy link
Collaborator Author

mjbvz commented Jan 3, 2017

Initial work for rejecting completion item promise was completed with #17098. We now need to surface these rejection message in some way and also look into adopting this pattern in other places.

@jrieken
Copy link
Member

jrieken commented Jan 5, 2017

For suggest I have something like this in mind - clearly it needs some UX hands @bgashler1, @stevencl . However, the idea is to show suggestion plus one or multiple messages explaining the result or the lack of a result

jan-05-2017 16-33-17

@mjbvz
Copy link
Collaborator Author

mjbvz commented Jan 5, 2017

@jrieken I like the concept. It doesn't block the user from using the feature while also clearly indicating that the results may not be complete.

Are you blocked by anything on the TS side?

@bgashler1
Copy link
Contributor

@jrieken I like this idea, too 👍 . I'll finesse the design of it and give you some mockups.

By the way, are we ever going to apply something like this to more than TypeScript? I feel like it might be nice to say "Fetching more suggestions..." to keep it more generic and intuitively worded (perhaps on hover a tooltip could specify the source it's searching: "Automatic type acquisition in progress").

@bgashler1 bgashler1 self-assigned this Jan 5, 2017
@jrieken
Copy link
Member

jrieken commented Jan 6, 2017

@mjbvz re #15209 (comment) - unsure, I have seconds thoughts about the API. I'll let it cock and a discussion on the other issue

@bgashler1 re #15209 (comment) - In theory this can show for any language and also multiple message can show. Worst, there might be different severities - errors/warning/info etc. Other samples could be:

  • go extension telling to install a certain tools for completions to work better
  • recommender installing you to install another extension for let's say better php IntelliSense etc

The message will be local to the languages (so go-message for go-file etc) but the overall design should be general purpose

@stevencl
Copy link
Member

Sorry for the delayed response.

I'm assuming that in regular usage we wouldn't often need to show this message since type acquisition would be fast enough that it would have completed even before the user attempts to bring up intellisense?

If that is the case, what are the scenarios in which we would need to show the message? Slow/no network connection? Need to load a large number of definition files?

I'm also assuming that the way it would work would be that the user sees a list, say just one with text suggestions. Once type acquisition is complete, the list is refreshed to show the new items. If the user had already selected an item in the original list, that selection is maintained with new items appearing above and below the selected item as appropriate.

Once the type acquisition is complete, does the message update to say so or does the whole message box just disappear?

Regarding the other usages such as telling the user to install another extension for better intellisense, we should think about our overall notification story (we don't have a good one at the moment). Since the suggestion list is transient we will likely need another way to tell the user about the likes of extensions that we recommend they install so that by the time they get around to performing the recommended action, they are able to easily retrieve the details of the notification/recommendation.

@jrieken
Copy link
Member

jrieken commented Jan 10, 2017

These are good question and I leave it up to @kieferrm and @mjbvz to fill in the blanks and correct me where I am wrong.

In short, the idea is to show something at the place at which the user is currently looking. The worry is that something like a status message at the bottom is too easy to miss. Samples are type acquisition but maybe also things like Install tool X for better IntelliSense. I am pretty confident such a message is harder to miss but I am also afraid that it will be abused because it is open for all extensions...

The lifecycle is not yet defined, but my likes go for a simple and stateless solution. That is, for a single invocation of IntelliSense that message shows, nothing updates while the session is active, and things start over when IntelliSense is request again (new session).

I am open for alternative suggestions and ideas, we should likely take this to the UX call. The problem we try to solve is that we are not very good when a certain operation fails, despite us knowing why it failed. One sample is IntelliSense but the same applies to 'Go to definition' which sometimes doesn't do anything, giving the impression of being unreliable etc. Our idea is to bring more information 'into the features' than always showing messages here and there.

@mjbvz
Copy link
Collaborator Author

mjbvz commented Jan 12, 2017

@jrieken Yes, that is my understanding of the expected behavior as well. The suggestion list would not be refreshed while it is displaying.

As currently designed, the warning message would only be shown while Typings are being acquired, such as when you open a new project or first import a module. TypeScript has done a lot of work around caching typing files to hopefully minimize how often we actually need to retrieve these typings file, but we do expect this to happen on the first open of large projects with many dependencies. Once the types are successfully installed, the message would go away.

We currently do not handle the case where typing acquisition has failed, and I think that any message we do show in that case should be actionable or link to more detailed errors information that we surface elsewhere. Perhaps could show those in the problems view, although then the impact of the error may not be clear to users. It's hard to know if the real impact of missing typings, and if we go with showing the errors in the suggestion popup, I think we would want to also provide a way for users to dismiss them. I can also see these errors and warnings being abused, or at least becoming annoying

@bgashler1
Copy link
Contributor

bgashler1 commented Jan 12, 2017

It sounds like we're reluctant to refresh the intelliSense session while it's open because it would be distracting to have new entries appear and have the list shift in a weird way. I can totally understand that, if that's the case.

Just thinking out loud: maybe upon completion the message updates into a selectable list item that says something like, "Reopen for newly available suggestions..." That way we avoid the appearance of an infinitely long acquisition cycle (positive feedback upon completion), we can communicate how to get these settings, and we provide a direct command to do that right away if desired. What do you guys think? Of course, in an ideal world everything (and the internet) would be so fast we would never have to show these messages.

@jrieken
Copy link
Member

jrieken commented Jan 12, 2017

The API challenge is that a message such as "Loading more types for more IntelliSense" is the sole result of that invocation. The provider cannot update it once it is out. More elaborate interactions requires a stateful API in which the provider/extension manages a separate status/progress object. Generally that's a little harder for extension writers to manage but definitely possible to implement.

@jrieken
Copy link
Member

jrieken commented Jan 17, 2017

To give us more time to figure out an inplace UX and to define the status API I wanna implement a solution that uses the proposed progress API to show a message while ATA is running. The UX is a spinning icon and a message in the status bar.

Question for @mjbvz and @kieferrm How do I test/trigger ATA? I started something in https://github.com/Microsoft/vscode/tree/joh/ata-progress but I have no clue how to make TypeScript send my those install-begin/end events...

@mjbvz
Copy link
Collaborator Author

mjbvz commented Jan 19, 2017

@jrieken To trigger ATA, you should just be able to add require('lodash') or import * as x from 'lodash' to a js file. This works for the most common packages at least. For other packages, I think you still have to list them in the package.json.

If the typings have been installed in the past, the cached version will be used. Try clearing the /Users/matb/Library/Caches/typescript/ folder and it should reinstall everything again. You can also check the package.json in that folder to see what typings have been installed.

@jrieken
Copy link
Member

jrieken commented Jan 19, 2017

This is what I propose for now:

jan-19-2017 12-30-12

@bgashler1 bgashler1 added this to the February 2017 milestone Jan 21, 2017
@bgashler1 bgashler1 removed this from the January 2017 milestone Jan 21, 2017
@bgashler1
Copy link
Contributor

Moving the rest of this to February after we have time to investigate more UX and API concerns.

@noinkling
Copy link

noinkling commented Feb 6, 2017

From a user perspective, I would definitely like to see some sort of explicit indicator that typing acquisition has succeeded or failed (and an easy to read message in the TS output pane or something). In my case ATA doesn't seem to work for some reason, but really the only indicator I have of this is that I don't get intellisense when working with certain packages. For the record, these are packages where the intellisense works fine if I install the typings manually. It would be nice to have a starting point for figuring out what's wrong, because currently all I get is No content available. messages in the TS output (after setting it to verbose).

@jrieken
Copy link
Member

jrieken commented Feb 15, 2017

We will use the status bar progress message for now.

@jrieken jrieken closed this as completed Feb 15, 2017
@jrieken jrieken added the verification-needed Verification of issue is requested label Feb 21, 2017
@roblourens
Copy link
Member

I see the status bar message, which was also there in 1.9. @mjbvz says nothing changed this month?

@roblourens roblourens added the verified Verification succeeded label Feb 24, 2017
@roblourens
Copy link
Member

I guess this was never verified originally, so anyway, consider it verified.

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 18, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
plan-item VS Code - planned item for upcoming verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

7 participants