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

Search page refactor #110

Merged
merged 10 commits into from
Oct 5, 2023
Merged

Search page refactor #110

merged 10 commits into from
Oct 5, 2023

Conversation

jamiefeiss
Copy link
Collaborator

The search page now uses the new search implementation outlined in RDFLib/prez#149.

Users can filter by a base class, containers, and can provide custom inbound & outbound filters.

image

Resolves #95, resolves #72.

@recalcitrantsupplant
Copy link
Collaborator

I've tested locally, looks good, only one minor issue:
As the current backend MVP search doesn't allow UNION across different filter conditions, rather it's an AND, if a user tries to search in two kinds of Container class, most likely results do not exist as instances of both Container class, so they'll get no results. For example restricting to search within datasets only yields a result:

image

Searching within a dataset and a vocabulary yields no result:

image

The assumption here is a user would expect when ticking multiple Container instances, results can come from within any instance, not that the result must exist in all of them.

I'd make the following recommendations for non-MVP enhancements:

  • tick all Container options and Containers by default - or have a special "select all" option at the top so it's clear everything is searched by default. When the query is sent to the backend, if it's a "select all" then the filter could be left off altogether, I think you've previously suggested this.
  • Make it clear which label properties/predicates are being searched. At present this is label, description, and provenance (yes, the backend still needs to expose this information ..)
  • Display the object class using a label rather than URI
  • Presumably searching within a Container should include all objects that are transitively within it - e.g. Features within a Dataset - something for us to discuss tomorrow. We'd need to update the backend to support this first though.

Copy link
Collaborator

@recalcitrantsupplant recalcitrantsupplant left a comment

Choose a reason for hiding this comment

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

see comments above

…avour searches with SearchBar; fixed query string re-rendering, renamed Advanced Search to Search
@jamiefeiss
Copy link
Collaborator Author

@recalcitrantsupplant thanks for the comments. What changes would you suggest for this MVP now? I could start by adding to the page that you can't select multiple container types.

@recalcitrantsupplant
Copy link
Collaborator

Just the one to restrict searching to one container type at present

@jamiefeiss
Copy link
Collaborator Author

Added pagination using RDFLib/prez#159

@edmondchuc edmondchuc merged commit 61772f6 into main Oct 5, 2023
1 check passed
@edmondchuc edmondchuc deleted the feature/search-refactor branch October 5, 2023 04:22
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.

Improve the search story Add search to listing pages
3 participants