Fix issues with facet badges #4057
Conversation
EreMaijala
commented
Nov 5, 2024
EreMaijala
commented
- Count badges for checkbox facets were on wrong level and not right-aligned properly.
- Height and top settings in sandal and sandal5 caused misaligned numbers in badges.
- Count badges for checkbox facets were on wrong level and not right-aligned properly. - Height and top settings in sandal and sandal5 caused misaligned numbers in badges.
There was a problem hiding this comment.
Merging other PRs created some conflicts here, which I have resolved for you to save time.
Just one question based on a code review; once that's answered, I'll take some time looking at this hands-on (and/or ask for @sturkel89's help).
themes/bootstrap3/js/facets.js
Outdated
| @@ -493,7 +493,7 @@ VuFind.register('sideFacets', function SideFacets() { | |||
| .done(function onGetSideFacetsDone(response) { | |||
| $.each(response.data.facets, function initFacet(facet, facetData) { | |||
| var containerSelector = typeof facetData.checkboxCount !== 'undefined' | |||
| ? '.checkbox-filter' : ':not(.checkbox-filter)'; | |||
| ? '.checkboxFilter ' : ':not(.checkbox-filter)'; | |||
There was a problem hiding this comment.
This is quite confusing. Is it intentional that you're using .checkboxFilter in one case but .checkbox-filter in the other? If so, maybe a comment would be in order to clarify that.
I pondered whether introducing the .checkbox-filters class from #4058 early might help, but actually I'm not convinced that it would -- .checkbox-filter vs. .checkbox-filters also raises questions about whether there might be an accidental typo... so I think in either case, a comment here would be justified.
There was a problem hiding this comment.
True, that was a bit confusing. Now replaced with .facet-group, which also aligns nicely with #4058.
|
@EreMaijala, the code looks good to me now, but I confess that I can't remember how to turn on counts in checkbox facets to test! Is that only supported by certain backends? Can you refresh my memory? I wonder if some improved documentation in the .ini files is needed too, if this is just a setting that I couldn't find... |
It's not possible without customization at the moment, but I think I can provide another PR to enable them. |
|
@demiankatz Meanwhile the easiest way to test this is to hack \VuFind\AjaxHandler\GetSideFacets::getCheckboxFacetCount to: |
