Merged
Conversation
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Prerendering
The category search page (
/category-search) had a UX issue: Loading the category properties took some time (around 200ms) because the database is hosted remotely. Even when a user is already on CatDat and navigates to the search page without entering anything yet, this delay is annoying. But the category properties are known already at build time (and do not change afterwards). So there is no point in loading them at this step, and we should just prerender the (initial) search page to avoid that. This has been done in this PR.Separate Search Results Page
But the search results cannot be prerendered. Hence, I have created a separate page for them. (This separation has already been done for the comparison page before.)
This means this PR introduces breaking URL changes. The category search result page is now located at
/category-search/results?satisfied...instead of/category-search?satisfied...UX Improvements
Also, the UX has been improved by writing "Searching..." on the search button while the search is being done. The same is done on the comparison page where it now says "Comparing..." on the button while the comparison is done in the backend. This fixes more or less #44.
Functors
All updates to the category search page have also been applied to the functor search page. I will address the massive code duplication (introduced in #12) in a separate PR. For now, I have already unified the backend code for the search by writing a search handler that works with both entities. But this is more of a POC since the final solution will definitely look different.
Other Updates
Follow-Up
When the user wants to do another search, they can click "Start new search" which navigates pack to the initial search page. But (a) this creates some friction, (b) the current selection is lost. Maybe it is better to either keep the selection or allow the user to do another search also on the search result page. I have tried both, didn't like it. But this can be improved in a future PR. Maybe it's also no big issue. I will check the feedback.