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

ES6 IntelliSense UX improvements #981

Merged
merged 22 commits into from
Jun 7, 2016
Merged

Conversation

mousetraps
Copy link
Contributor

After auto-dts acquisition, the following InfoBar will appear in solution explorer to help educate the user and increase discoverability of the feature.
image

Clicking on the "typings folder" link will open a web page that includes more information about what's going on under the covers.

Clicking on the "Customize..." button will open the relevant options page, which has been reconstructed to be conditional upon the selected IntelliSense mode. ES6 will use the new Salsa language service, and ES5 will use the old Node.js static analysis engine.

ES6 Mode
image

ES5 Mode
image

Additional Considerations

  • InfoBar design
    • Consistent with VS UI guidelines
    • We do not want to block the user’s flow during project creation, so we’ve gone with an in-context infobar with an option to customize the experience rather than asking for confirmation. Anecdotally, most people are okay with this experience, and the action is also easy enough to undo (simply delete the folder)
    • Also considered adding a “Don’t show again” button to the info bar, but it’s a small space and cluttered things up a bit much.
    • It would be annoying for the infobar to show up every time typings are downloaded, so it will only show up when the folder is newly created. In every other case, it just prints out to the output window / status bar.
    • The message chosen for the info bar was a balance between informativeness and length (to minimize chance of wrapping over to line Code improperly indents after if block inside inline callback function #3)
    • Also considered infobar in editor rather than solution explorer, and the upside is that we wouldn’t be as limited with length of text, but the downside is that it can be quite jarring because the primary focus is on the editor, and also made things appear quite cluttered.
    • Selected NewFolder glyp because it best conveys the thing that the user cares about – which is that a new folder was added to their project.
  • Options design
    • When ES5 is selected, a warning will appear. Right now the warning appears below the combo box to inform the user that it’s been deprecated. The horizontal line and fixed height serve to make the reflow less jarring when a different IntelliSense mode is selected.
    • Quick IntelliSense will be selected by default when moving from ES6->ES5 mode (rather than full intellisense). All other options are retained when switching between modes.
    • running tsd with --save is now off by default so that we don't unnecessarily clutter up the user's project.

What's missing?

  • currently all the links point to the same page - we should create fwlinks for each, so that we have a little more flexibility
  • still need to update docs
  • was intending to synchronize formatting options between TS and Node.js, but it looks like that's not nearly as simple as anticipated, and we may need to reconsider our approach here.

get {
int max;
// The Max Value is described by 'Max' instead of 'Int32.MaxValue'
if (_analysisLogMax.Text == "Max") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why?

[nit] Also, extract Max to a constant if it is going to be used.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, see this was moved from other file. Fine leaving it as is.

@mjbvz
Copy link
Contributor

mjbvz commented Jun 6, 2016

Where any considerations made for typings acquisition being disabled in VS15? I'm fine merging this in before the VS15 experience is perfect since we don't have a release of that scheduled soon, but we should track this as a future work item.

Also, should #if DEV14 guards be used anywhere to explicitly prevent new logic or classes from being used in dev15?

}
return _instance;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] Prefer Lazy to encapsule this pattern.

@mjbvz
Copy link
Contributor

mjbvz commented Jun 6, 2016

The change looks great overall and should help users a lot.

I added a few nit comments and had a few questions/notes, but there were no major problems in the code. Let's just make sure we are on the same page regarding the VS15 and open any new work items for that before checking this in. We can create another WI for updating the documentation too, since that step shouldn't block this check in but must be done before the 1.2 release.

Also, if you are still looking into synchronizing the TS and Node formatting, please do that in another PR so we can keep this one more focused.

@mjbvz mjbvz merged commit 63479ff into microsoft:master Jun 7, 2016
@mousetraps mousetraps mentioned this pull request Jun 8, 2016
23 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants