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

feat(compass-editor, compass-query-bar): provide auto completed query history matching user input COMPASS-8018 #6040

Conversation

VivianTNT
Copy link
Contributor

@VivianTNT VivianTNT commented Jul 17, 2024

Description

COMPASS-8018

The query history autocompleter will still show up on focus but now users will also be able to type to match their input. The new autocompleter has 2 sections, with the original base completions for fields on the top and query history completions on the bottom (if any).
On focus:
image
Both types of completions:
image
Just saved queries:
image

Checklist

  • New tests and/or benchmarks are included
  • Documentation is changed or added

Motivation and Context

  • Bugfix
  • New feature
  • Dependency update
  • Misc

Open Questions

When a query is currently in an editor and the cursor is focused on it, pressing reset will trigger onFocus() for that particular editor and becomes unfocused after the {} and autocompleter are automatically triggered.

Dependents

COMPASS-8017

Types of changes

  • Backport Needed
  • Patch (non-breaking change which fixes an issue)
  • Minor (non-breaking change which adds functionality)
  • Major (fix or feature that would cause existing functionality to change)

@github-actions github-actions bot added the feat label Jul 17, 2024
@VivianTNT VivianTNT added the feature flagged PRs labeled with this label will not be included in the release notes of the next release label Jul 17, 2024
min: number,
max: number
): number {
if (max === min) return 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

this looks like unexpected input. returning a number could make such input go unnoticed, and 0 might be out of the allowed scale - so it would be safer to throw

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I returned 0 because in the context of the autocompleter, originalScaleMin = originalScaleMax only when there is 1 lastExecuted time, in which case it doesn't matter how the autocompleter ranks the completion. The boost property in the options expects numbers between -99 and 99, so I could probably return any number between those.

For the sake of making the input noticed, do you think it would be a better idea to throw an error in the function and then catch the error within the autocompleter options itself instead of handling it in the function?

Copy link
Member

@Anemy Anemy left a comment

Choose a reason for hiding this comment

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

Nice, left a comment or two on the recent changes. Looking good!

@VivianTNT VivianTNT force-pushed the COMPASS-8018-provide-auto-completed-query-history-matching-user-input branch from 3509baa to e2cb578 Compare July 19, 2024 19:30
Copy link
Member

@Anemy Anemy left a comment

Choose a reason for hiding this comment

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

Nice tests! Code looks good, left a comment on what we're autocompleting. We might want to match a bit less than we currently do.

@VivianTNT VivianTNT merged commit 5ba1566 into main Jul 22, 2024
30 checks passed
@VivianTNT VivianTNT deleted the COMPASS-8018-provide-auto-completed-query-history-matching-user-input branch July 22, 2024 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat feature flagged PRs labeled with this label will not be included in the release notes of the next release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants