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

LWS-244: Filter sort #1103

Merged
merged 10 commits into from
Sep 19, 2024
Merged

LWS-244: Filter sort #1103

merged 10 commits into from
Sep 19, 2024

Conversation

jesperengstrom
Copy link
Contributor

@jesperengstrom jesperengstrom commented Sep 13, 2024

Description

Tickets involved

LWS-244

Solves

Allowing the user to sort individual filter groups as well as some general changes to facet/filter section.

Summary of changes

Each filter group (with the exception of other) now gets a sort select element on expansion. The sorting is done in the client, and is persisted after navigation/reload by setting a cookie using the js-cookie package. By using cookies (and not localStorage) the setting is available during SSR, which is probably also needed with future user settings until we save them in a db.

The cookie is retrieved in the hooks handle function and passed to the frontend in the server load functions. Quite the roundtrip, but i've not come up with a more elegant way to do it...

Other changes

  • Use the details/summary elements for the filter panel (in line with rest of the app) to allow expanding the sections without js enabled.
  • Added an automatic axe a11y check when filter sections are expanded. This detected contrast violations for the discriminator text which i've made darker.
  • Replaced the ☑ char with icons for better visibility
  • Some other styling tweaks
  • Added tests for sort functionality

@jesperengstrom jesperengstrom marked this pull request as ready for review September 17, 2024 06:50
@jesperengstrom jesperengstrom changed the title Draft: LWS-244: Filter sort LWS-244: Filter sort Sep 17, 2024
Copy link
Contributor

@olovy olovy left a comment

Choose a reason for hiding this comment

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

Nice! LGTM!


Some things I would like see in a later PR:

  • Suitable default sort order for each facet group (should maybe be a backend config?)

Things to discuss later:

  • Some/top/all? facet groups should be expanded by default?
  • Some facet groups might work better as top N + alphabetically, e.g. Language: Swedish, English, German, Afrikaans, Akkandian, Albanian...

@jesperengstrom
Copy link
Contributor Author

Some things I would like see in a later PR:

  • Suitable default sort order for each facet group (should maybe be a backend config?)

I actually had this implemented ('special' sorting for groups was a var in the constants file) but it was then removed in discussion with Ebou since users can achieve the same thing if they want to. Don't know if that is a correct conclusion.

Let's merge this for testing and see what can be improved.

@jesperengstrom jesperengstrom merged commit bac4b79 into develop Sep 19, 2024
2 checks passed
@jesperengstrom jesperengstrom deleted the feature/LWS-244-filter-sort branch September 19, 2024 07:55
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.

2 participants