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

Improve ux experience through auto completer #2483

Merged
merged 13 commits into from
Sep 6, 2022
Merged

Conversation

DidierRLopes
Copy link
Collaborator

Give it a go

@DidierRLopes DidierRLopes added the feat XL Extra Large feature label Sep 2, 2022
@deeleeramone
Copy link
Contributor

deeleeramone commented Sep 2, 2022

/economy/index after the first index is selected, autocomplete wants to add a second index to the chart when the - is entered after the first choice.

Screen Shot 2022-09-02 at 9 31 42 AM

Screen Shot 2022-09-02 at 9 33 25 AM

Using the -i flag under /economy/index brings up choices that should be activated after space is entered in the command string as opposed to immediately being triggered by the single dash.

Screen Shot 2022-09-02 at 9 34 08 AM

Screen Shot 2022-09-02 at 9 36 17 AM

Also noteworthy is that double-dashes do not present any choices.

@DidierRLopes
Copy link
Collaborator Author

/economy/index after the first index is selected, autocomplete wants to add a second index to the chart when the - is entered after the first choice.

Screen Shot 2022-09-02 at 9 31 42 AM Screen Shot 2022-09-02 at 9 33 25 AM

Using the -i flag under /economy/index brings up choices that should be activated after space is entered in the command string as opposed to immediately being triggered by the single dash.

Screen Shot 2022-09-02 at 9 34 08 AM Screen Shot 2022-09-02 at 9 36 17 AM

Also noteworthy is that double-dashes do not present any choices.

I only tested the macro economy. Will look into what's going on here. Thanks!

@DidierRLopes
Copy link
Collaborator Author

/economy/index after the first index is selected, autocomplete wants to add a second index to the chart when the - is entered after the first choice.

Screen Shot 2022-09-02 at 9 31 42 AM Screen Shot 2022-09-02 at 9 33 25 AM

Using the -i flag under /economy/index brings up choices that should be activated after space is entered in the command string as opposed to immediately being triggered by the single dash.

Screen Shot 2022-09-02 at 9 34 08 AM Screen Shot 2022-09-02 at 9 36 17 AM

Also noteworthy is that double-dashes do not present any choices.

Notes:

The index only had autocomplete for "index WORD" hence why there are no "--" and "-" args autocompleters - the behaviour is the same.

This auto-completer is designed to need a "--" flag to proceed to autocomplete, hence why results are sucky with "-". Having both would increase time by 2x, so we will only support an autocomplete for one.

@DidierRLopes DidierRLopes marked this pull request as draft September 2, 2022 16:57
@deeleeramone
Copy link
Contributor

This auto-completer is designed to need a "--" flag to proceed to autocomplete, hence why results are sucky with "-". Having both would increase time by 2x, so we will only support an autocomplete for one.

By this, do you mean that autocomplete will support only "--", and not "-"?

@DidierRLopes
Copy link
Collaborator Author

This auto-completer is designed to need a "--" flag to proceed to autocomplete, hence why results are sucky with "-". Having both would increase time by 2x, so we will only support an autocomplete for one.

By this, do you mean that autocomplete will support only "--", and not "-"?

Likely yes. The reason being for performance (speed), not because we can't make it happen :)

@DidierRLopes
Copy link
Collaborator Author

@deeleeramone try it out now. I fixed the entire menu to have autocomplete for "--".

@jmaslek @piiq check the implementation on the economy menu if u have some time

@deeleeramone
Copy link
Contributor

It seems to work, but with a caveat being after the first command is run, the functions in the menu no longer trigger autocomplete. so, if you run overview once, the menu has the functions autocomplete, but the second time it does not and is only triggered when -- is deployed.

Screen Recording 2022-09-02 at 9 44 34 PM

This is more of a blocker than above, @DidierRLopes: map --type brings up the choices only for overview --type.

Screen Shot 2022-09-02 at 10 02 45 PM

@DidierRLopes
Copy link
Collaborator Author

It seems to work, but with a caveat being after the first command is run, the functions in the menu no longer trigger autocomplete. so, if you run overview once, the menu has the functions autocomplete, but the second time it does not and is only triggered when -- is deployed.

Screen Recording 2022-09-02 at 9 44 34 PM Screen Recording 2022-09-02 at 9 44 34 PM

This is more of a blocker than above, @DidierRLopes: map --type brings up the choices only for overview --type.

Screen Shot 2022-09-02 at 10 02 45 PM

Very nice catch. This is not easy to trigger, it happens when you delete the command that you had first input, I am going to fix this - thanks for spotting it.

@DidierRLopes
Copy link
Collaborator Author

@deeleeramone both those cases where caused by the fact of you going back and changing first argument, I didn't account for that behaviour.

It should now be taken care of. Let me know if it works well now!

@jmaslek
Copy link
Collaborator

jmaslek commented Sep 3, 2022

This is definitely a sweet improvement.

Is a little buggy. This behavior changed at some point. Seems to be when i was in the middle of a completer than I do a backspace. (Confirms this happens when backspacing)

Screen Shot 2022-09-03 at 2 43 20 PM

Also if I start to type, the autocompleter doesnt know where I am in the list.

Screen Shot 2022-09-03 at 2 45 46 PM

@deeleeramone
Copy link
Contributor

industry shows as a choice for a function and not an argument.

Screen Shot 2022-09-03 at 1 26 48 PM

bigmac --country doesn't populate with countries, and instead presents column names as choices.

Screen Shot 2022-09-03 at 1 41 33 PM

ycrv --country is not able to handle the spaces where a country name is two words.
Screen Shot 2022-09-03 at 1 43 55 PM

@DidierRLopes
Copy link
Collaborator Author

This is definitely a sweet improvement.

Is a little buggy. This behavior changed at some point. Seems to be when i was in the middle of a completer than I do a backspace. (Confirms this happens when backspacing)

Screen Shot 2022-09-03 at 2 43 20 PM

Also if I start to type, the autocompleter doesnt know where I am in the list.

Screen Shot 2022-09-03 at 2 45 46 PM

This is with my last change, messed up that end. Will look into this.

@DidierRLopes
Copy link
Collaborator Author

@jmaslek and @deeleeramone . Both should be fixed now!

@DidierRLopes DidierRLopes marked this pull request as ready for review September 4, 2022 00:46
@DidierRLopes
Copy link
Collaborator Author

This is as good as it gets if not even @deeleeramone can find any issue

Screenshot 2022-09-06 at 22 54 32

@DidierRLopes DidierRLopes merged commit e6eafc0 into main Sep 6, 2022
@Chavithra Chavithra deleted the autocomplete branch September 20, 2022 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat XL Extra Large feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants