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

First PR: Adding jump to nth autocomplete suggestion via keyboard shortcut #58670

Closed
wants to merge 2 commits into from

Conversation

helsont
Copy link

@helsont helsont commented Sep 14, 2018

Link to issue: #52549

Hi! This is my first PR and I'd like to get feedback on my general approach before I dig any deeper.

I know that the particular configurations of my key bindings can be improved and I don't know what case State.Details represents. Feedback and help appreciated here!

More generally, is the new function selectNthSuggestion ok? What about the for loop to go through and add the bindings? How's the id for the key binding?

Please be as honest as possible as it's my first PR and I'd like to learn quickly!

@msftclas
Copy link

msftclas commented Sep 14, 2018

CLA assistant check
All CLA requirements met.

@matklad
Copy link

matklad commented Oct 2, 2018

👍

Perhaps it makes sense not only to select the suggestion, but to insert it right away?

@ramya-rao-a ramya-rao-a requested a review from jrieken November 19, 2018 03:48
@ramya-rao-a ramya-rao-a assigned jrieken and unassigned ramya-rao-a Nov 19, 2018
@jrieken jrieken added ux User experience issues suggest IntelliSense, Auto Complete labels Nov 21, 2018
@jrieken
Copy link
Member

jrieken commented Nov 21, 2018

@helsont this will take a while tbh. @ramya-rao-a is leaving the team and things are parked here until a new owner is found

@octref octref self-requested a review September 9, 2019 21:37
@octref octref added this to the September 2019 milestone Sep 9, 2019
@jrieken jrieken assigned octref and unassigned jrieken Sep 10, 2019
@octref
Copy link
Contributor

octref commented Sep 23, 2019

@helsont Are you still interested in carrying this on?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
suggest IntelliSense, Auto Complete ux User experience issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants