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

Refactor works mobile filter #326

Merged
merged 5 commits into from
Apr 23, 2024
Merged

Refactor works mobile filter #326

merged 5 commits into from
Apr 23, 2024

Conversation

damisul
Copy link
Collaborator

@damisul damisul commented Apr 20, 2024

Refactored works/browse page to use same html form for desktop and mobile views.
Refactored several partials to share more code between authors and works browse pages.

damisul added 3 commits April 21, 2024 03:20
…folder.

Modified labels retrieval for checkboxes.
Rewritten copyright filter in author browse page using checkbox_group partial.
Added filter/languages partial for languages filter
@damisul damisul requested a review from abartov April 20, 2024 22:24
@damisul damisul force-pushed the refactor_works_mobile_filter branch from 08bddd4 to 0ad5884 Compare April 20, 2024 23:00
@damisul damisul force-pushed the refactor_works_mobile_filter branch 2 times, most recently from e660f6b to a930694 Compare April 20, 2024 23:16
…ilter and moved this partial to shared/filters folder.
@damisul damisul force-pushed the refactor_works_mobile_filter branch from a930694 to f9066f1 Compare April 20, 2024 23:18
.by-card-v02.text-sorting#sort_filter_panel{style: 'display:show'}
= render partial: 'browse_filters'
.col-12.col-lg-8
.by-card-v02.text-sorting#sort_filter_panel{ style: 'display:none' }
Copy link
Owner

Choose a reason for hiding this comment

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

shouldn't this be conditional upon filters.empty? too?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks, @abartov a good catch!
But we cannot decide if element should be shown or not on server side, as we need to know if this is a mobile view or desktop.
So I've added JS code to handle this.

Also added couple of other adjustements:

  • browse_intro snippet should not be shown on mobile at all (to copy old behaviour)
  • added scrolling to the top of the list when mobile filter is applied.

And modified authors browse page accordingly.

…thors and mobile pages: they should not be shown on mobile at all.

Added scrolling to the top of the list when mobile filter is applied.
@damisul damisul requested a review from abartov April 23, 2024 18:19
@abartov abartov merged commit c131871 into master Apr 23, 2024
7 checks passed
@abartov abartov deleted the refactor_works_mobile_filter branch April 23, 2024 19:12
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