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

Add query builder header in embedded dialog #3850

Merged
merged 13 commits into from
Sep 29, 2023
Merged

Conversation

CarolineDenis
Copy link
Contributor

Fixes #3830

@maxpatiiuk
Copy link
Member

@specify/ux-testing when testing, please be extra careful to also test the save/save as buttons
test in different places where embedded query appears: Search Dialog, Meta->Edit History, (for a pick list record) Meta->Pick List Usages

@grantfitzsimmons grantfitzsimmons added this to the 7.9.1 milestone Jul 31, 2023
@CarolineDenis
Copy link
Contributor Author

CarolineDenis commented Aug 2, 2023

@maxpatiiuk all the query builder are synchronized atm, if we click Hide Mapping in one query it will be the case for all the queries. Is that what we want?

@melton-jason
Copy link
Contributor

melton-jason commented Aug 2, 2023

@maxpatiiuk all the query builder are synchronized atm, if we click Hide Mapping in one query it will be the case for all the queries. Is that what we want?

I believe that would also be a question to ask @specify/ux-testing 😉

Copy link
Member

@maxpatiiuk maxpatiiuk left a comment

Choose a reason for hiding this comment

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

remove commented out code

@maxpatiiuk
Copy link
Member

maxpatiiuk commented Aug 3, 2023

@maxpatiiuk all the query builder are synchronized atm, if we click Hide Mapping in one query it will be the case for all the queries. Is that what we want?

Fine with me either way.
If you want, you can store in separate local storage keys depending on whether query builder is embedded or not - as most users would likely want to see the mapping explorer in the full query builder, but might want to hide it when in the dialog if on a smaller screen

@grantfitzsimmons grantfitzsimmons changed the base branch from v7.9-dev to production September 26, 2023 02:44
@CarolineDenis CarolineDenis removed the request for review from melton-jason September 26, 2023 15:05
Copy link

@bronwyncombs bronwyncombs left a comment

Choose a reason for hiding this comment

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

When saving a query made in an embedded dialog (not via stats) it will prompt you to leave the current form and open the saved query in the main query builder.

It should just save the query and not show this dialog. Same applies to Save As button.

Screen.Recording.2023-09-29.at.10.03.26.AM.mov

Comments:

Embedded dialog header on stats page looks good! Switching to basic view reduces the width of the dialog, but I think this is fine.

Embedded and full query builders in sync so detailed/basic view is carried over

@CarolineDenis
Copy link
Contributor Author

Decision:
Keep "Basic View" and "Hide Field Mapper" bins in embedded header dialog. Remove "Save Query" btn from it

Triggered by c0e2ccb on branch refs/heads/issue-3830
Copy link

@bronwyncombs bronwyncombs left a comment

Choose a reason for hiding this comment

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

Looks good! Save/Save As issue resolved.

@CarolineDenis CarolineDenis merged commit 987ecb8 into production Sep 29, 2023
9 checks passed
@CarolineDenis CarolineDenis deleted the issue-3830 branch September 29, 2023 17:25
@maxpatiiuk
Copy link
Member

For future reference, we could bring back the save button, or fix #3000

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Add the ability to switch from detailed to basic view in query dialogs
5 participants