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): show query history in autocomplete COMPASS-8017 #5995

Merged
merged 15 commits into from
Jul 10, 2024

Conversation

VivianTNT
Copy link
Contributor

Description

COMPASS-8017

When the query bar is on focus and is either empty or {}, the autocompleter pops up with a list of all saved queries (recent and favorites). It can also be triggered using ctrl-space. The queries are ranked by their last executed time (most recent at the top). When an autocomplete option is pre-selected (i.e. using arrow keys but not clicked on yet), a side popup appears which renders the full query similar to how the current query history popup does it. Each query property is displayed and labeled if it is part of the current query.
image
When a query is clicked on or tab is pressed, the query's properties are applied to the option editor exactly like how it works currently with the query history popup.
image
All of this is also applicable to the rest of the textboxes in the query option editor.

Checklist

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

Motivation and Context

  • Bugfix
  • New feature
  • Dependency update
  • Misc

Open Questions

Dependents

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)

@VivianTNT VivianTNT added the feat label Jul 3, 2024
@VivianTNT VivianTNT requested a review from Anemy July 3, 2024 15:35
Copy link
Collaborator

@gribnoysup gribnoysup left a comment

Choose a reason for hiding this comment

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

Overall looks very good! Left a few notes on the implementation, we are changing the business logic of the store in this PR and it would be good to account for this change in other parts of the query-bar that are also using this state

packages/compass-query-bar/src/stores/query-bar-reducer.ts Outdated Show resolved Hide resolved
packages/compass-query-bar/src/components/query-bar.tsx Outdated Show resolved Hide resolved
}) => {
useEffect(() => {
loadSavedQueries();
Copy link
Collaborator

@gribnoysup gribnoysup Jul 4, 2024

Choose a reason for hiding this comment

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

We now have two separate places in UI that trigger loading of queries on mount, this is generally not a very conventional way of doing this. Now that you changed the saveRecentQuery to reload the stored values after the update, we can remove loadSavedQueries from here and from the QueryHistoryButtonPopover and add a dispatch for loading queries in the plugin activate method. After the initial load the refresh you added should make sure that the state is in sync with the storage

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.

lgtm! nice!!!

@VivianTNT VivianTNT merged commit 00a8966 into main Jul 10, 2024
28 of 30 checks passed
@VivianTNT VivianTNT deleted the COMPASS-8017-show-query-history-in-autocomplete branch July 10, 2024 16:46
@Anemy Anemy added feature flagged PRs labeled with this label will not be included in the release notes of the next release and removed release notes labels Jul 18, 2024
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