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

Synchronize search input with query #388

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

stephenkilbourn
Copy link

@stephenkilbourn stephenkilbourn commented Nov 9, 2023

Fixes #302

This PR is intended to synchronize the filter options on the /search route with url parameters.

searchParamRedo

  1. parameters in the URL are saved as a user makes changes
  2. loading the /search route with URL parameters submits the form
  3. Because changing the form does update the URL parameters, hitting reload would actually submit the form, as well.
  4. Clicking reset sets the form back to the default state and clears the url params. It is not returning to the state first navigated to by a user.
  5. The areaselect component doesn't handle passing in a pre-selected area. Without that, it was hard for a user to determine what the spatial extent in the search was. My route to solve this was to add a new BBEntry component that allows the entry of [x_min, y_min, x_max, y_max]. If a shared url has spatial extent defined, then the bounding box selection will be here. The default behavior remains using the bounding box. This new component should also help accessibility, but a follow up task to update the areaSelect to accept an entry might be nice.

Copy link
Collaborator

@m-mohr m-mohr left a comment

Choose a reason for hiding this comment

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

Thanks for the PR.

Could you please separate the PRs into two parts:

  1. Manual BBox entry
  2. Serarch input sync with query

I think we can more easily merge 1, but 2 is more difficult as it doesn't cater for all cases yet (queryables, collection search) and I fear that merging 2 in the current state will lead to bug reports that the sync is not complete.

@stephenkilbourn stephenkilbourn changed the title Synchronize search input with query & add manual BBox entry Synchronize search input with query Nov 10, 2023
@stephenkilbourn
Copy link
Author

@m-mohr - I updated this PR to just address the Search input sync with query, and I'll open the separate one for the BBoxEntry. I'll dig into the queryables and anything else I missed, as well. My intro to this browser didn't show the queryable portion, so I wasn't aware to be testing that out.

@m-mohr
Copy link
Collaborator

m-mohr commented Nov 10, 2023

Thanks.
For the sync with query part, I also started work on that some time ago but there were so many small technical details to synchronize that in the end I couldn't make it work completely with reasonable effort. And this was even before Collection Search was implemented. See main...uistate-simple for details.
If you want to invest the time, I'd appreciate it. But on the other hand, be aware that it may be a can of worms...

@stephenkilbourn
Copy link
Author

Thanks for the pointer. This was suggested to me as a good first task if I was able to learn some Vue. I'll dig into what you linked and see if I can make reasonable progress.

@m-mohr
Copy link
Collaborator

m-mohr commented Nov 10, 2023

Oh! The search box is a good one for "beginners" I guess, but the UI state for search is definitively a very complex issue I wouldn't recommend in this case.

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

Successfully merging this pull request may close these issues.

Synchronize search input with query
2 participants