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

[discover] Query editor UI clean up #7896

Merged
merged 3 commits into from
Aug 29, 2024

Conversation

abbyhu2000
Copy link
Member

@abbyhu2000 abbyhu2000 commented Aug 28, 2024

Description

  • register query reference control when registering the language
  • make toggle and saved query management part of editor, not query control

Issues Resolved

Screenshot

Testing the changes

Changelog

  • fix: Query editor UI clean up

Check List

  • All tests pass
    • yarn test:jest
    • yarn test:jest_integration
  • New functionality includes testing.
  • New functionality has been documented.
  • Update CHANGELOG.md
  • Commits are signed per the DCO using --signoff

Copy link
Contributor

❌ Empty Changelog Section

The Changelog section in your PR description is empty. Please add a valid changelog entry or entries. If you did add a changelog entry, check to make sure that it was not accidentally included inside the comment block in the Changelog section.

const defaultEditor = createEditor(
SingleLineInput,
SingleLineInput,
[createDefaultLanguageReference()],
Copy link
Member

Choose a reason for hiding this comment

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

i see from my earlier comment.

This is already a lot cleaner so def want this in but I think we can think bigger similar to @joshuali925 stuff with the query header extension being a container this could be the same. And I think the naming will help because i was a little confused before.

We could make this more generic. Will comment in my overall thread.

kavilla
kavilla previously approved these changes Aug 28, 2024
Copy link
Member

@kavilla kavilla left a comment

Choose a reason for hiding this comment

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

Looks a lot cleaner. Thank you so much.

I think we can make it more generic. This is more like query editor actions that developers can register to. Or we can just have a container of queryEditorActions which then lets developers pass any or none in.

But this goes to ensuring the editor is exposed correctly to be reused with the correct state.

Not blocking but i think post 2.17 we should revisit and making it more generic that the editors are exposed and registered completely in the language.

@kavilla
Copy link
Member

kavilla commented Aug 28, 2024

can you make sure the changelog section is accurate

joshuali925
joshuali925 previously approved these changes Aug 28, 2024
opensearch-changeset-bot bot added a commit to abbyhu2000/OpenSearch-Dashboards that referenced this pull request Aug 28, 2024
@opensearch-changeset-bot opensearch-changeset-bot bot dismissed stale reviews from joshuali925 and kavilla via 0838136 August 28, 2024 22:53
abbyhu2000 pushed a commit to abbyhu2000/OpenSearch-Dashboards that referenced this pull request Aug 28, 2024
kavilla
kavilla previously approved these changes Aug 28, 2024
@kavilla kavilla changed the title Query editor UI clean up [discover] Query editor UI clean up Aug 28, 2024
joshuali925
joshuali925 previously approved these changes Aug 28, 2024
abbyhu2000 pushed a commit to abbyhu2000/OpenSearch-Dashboards that referenced this pull request Aug 28, 2024
@abbyhu2000 abbyhu2000 dismissed stale reviews from joshuali925 and kavilla via 425335f August 28, 2024 23:18
@abbyhu2000 abbyhu2000 merged commit fd8af76 into opensearch-project:main Aug 29, 2024
65 of 67 checks passed
@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch-Dashboards/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch-Dashboards/backport-2.x
# Create a new branch
git switch --create backport/backport-7896-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 fd8af76ae1327f96f7b7058f24296900bd5817b0
# Push it to GitHub
git push --set-upstream origin backport/backport-7896-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch-Dashboards/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-7896-to-2.x.

abbyhu2000 added a commit to abbyhu2000/OpenSearch-Dashboards that referenced this pull request Aug 29, 2024
* follow up

Signed-off-by: abbyhu2000 <[email protected]>

* address comments

Signed-off-by: abbyhu2000 <[email protected]>

* Changeset file for PR opensearch-project#7896 created/updated

---------

Signed-off-by: abbyhu2000 <[email protected]>
Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
abbyhu2000 added a commit that referenced this pull request Aug 29, 2024
* follow up



* address comments



* Changeset file for PR #7896 created/updated

---------

Signed-off-by: abbyhu2000 <[email protected]>
Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants