|
Description
Lucas Gass (lukeg)
2020-04-29 15:43:38 UTC
Created attachment 103957 [details] [review] Bug 25314: Make OPAC facets collapse TO TEST: 1. Apply patch 2. Regenerate CSS (yarn build --view opac) 3. DO a search and look at the facets. They should be collapsed. 4. Try mousing over a facet heading and click, that list should appear. 5. Try clicking the heading again to make th facet disappear. 6. Play around with different limiters and make sure eveything works. Created attachment 103958 [details] [review] Bug 25314: Make OPAC facets collapse TO TEST: 1. Apply patch 2. Regenerate CSS (yarn build --view opac) 3. DO a search and look at the facets. They should be collapsed. 4. Try mousing over a facet heading and click, that list should appear. 5. Try clicking the heading again to make th facet disappear. 6. Play around with different limiters and make sure eveything works. This looks good! I wonder if we need a way for the library to define which facet(s) should be expanded by default? Created attachment 103997 [details] [review] Bug 25314: Make OPAC facets collapse TO TEST: 1. Apply patch 2. Regenerate CSS (yarn build --view opac) 3. DO a search and look at the facets. They should be collapsed. 4. Try mousing over a facet heading and click, that list should appear. 5. Try clicking the heading again to make th facet disappear. 6. Play around with different limiters and make sure eveything works. 7. Check aria labels look correct and you can tab through facets without a keyboard. Owen, Would this be a system preference where libraries could define which facets should be expanded by default? I anticipate that a lot of people will miss the 'by default expanded' facets. Created attachment 107447 [details] Failed QA: facets showing "<<<< HEAD" Tested this patch. Confirmed that without the patch, the search facets were all showing uncollapsed. I added the patch, and although the facets are now collapsed, they are also not appearing correctly. Each facet is displaying multiple times, with the text size changing each time it appears. There is also "======" and "<<<<<< HEAD" showing mixed in with the facets. See attachment "Bug25314.1" (In reply to madamyk from comment #7) > Created attachment 107447 [details] > Failed QA: facets showing "<<<< HEAD" > > Tested this patch. Confirmed that without the patch, the search facets were > all showing uncollapsed. > > I added the patch, and although the facets are now collapsed, they are also > not appearing correctly. Each facet is displaying multiple times, with the > text size changing each time it appears. There is also "======" and "<<<<<< > HEAD" showing mixed in with the facets. See attachment "Bug25314.1" Thanks for testing, this patch needs to be rebased for master. I will do so. Created attachment 107448 [details] [review] Bug 25314: Make OPAC facets collapse TO TEST: 1. Apply patch 2. Regenerate CSS (yarn build --view opac) 3. DO a search and look at the facets. They should be collapsed. 4. Try mousing over a facet heading and click, that list should appear. 5. Try clicking the heading again to make th facet disappear. 6. Play around with different limiters and make sure eveything works. 7. Check aria labels look correct and you can tab through facets without a keyboard. Created attachment 107450 [details] [review] Bug 25314: Make OPAC facets collapse TO TEST: 1. Apply patch 2. Regenerate CSS (yarn build --view opac) 3. DO a search and look at the facets. They should be collapsed. 4. Try mousing over a facet heading and click, that list should appear. 5. Try clicking the heading again to make th facet disappear. 6. Play around with different limiters and make sure eveything works. 7. Check aria labels look correct and you can tab through facets without a keyboard. Created attachment 107451 [details] [review] Bug 25314: Add system pref to control expanding/collapsing facets TO TEST: 1. Apply patch 2. Regenerate CSS (yarn build --view opac), updatedatabase and restart_all 3. DO a search and look at the facets. They should be collapsed. 4. Try mousing over a facet heading and click, that list should appear. 5. Try clicking the heading again to make th facet disappear. 6. Play around with different limiters and make sure eveything works. 7. Check aria labels look correct and you can tab through facets without a keyboard. 8. Find the system pref OPACFacetsCollapseByDefault in OPAC > Features 9. Try toggling different facets and make sure they all expland/collapse correctly Created attachment 107452 [details] [review] Bug 25314: Add system pref to control expanding/collapsing facets TO TEST: 1. Apply patch 2. Regenerate CSS (yarn build --view opac), updatedatabase and restart_all 3. DO a search and look at the facets. They should be collapsed. 4. Try mousing over a facet heading and click, that list should appear. 5. Try clicking the heading again to make th facet disappear. 6. Play around with different limiters and make sure eveything works. 7. Check aria labels look correct and you can tab through facets without a keyboard. 8. Find the system pref OPACFacetsCollapseByDefault in OPAC > Features 9. Try toggling different facets and make sure they all expland/collapse correctly Doesn't look like this has been tested so I'm not sure why the status was changed to 'Signed-off', reseting this to 'Needs Signoff' (In reply to Lucas Gass from comment #13) > Doesn't look like this has been tested so I'm not sure why the status was > changed to 'Signed-off', reseting this to 'Needs Signoff' I tested this bug. It works. Ivan, In that case you need to also upload the signed-off patch as attachment and mark the initial one as obsolete. We can take care of adding the sign off line in QA this time, but signing off patches is easy with the sandboxes if you want to give it a try: https://wiki.koha-community.org/wiki/Sandboxes There are some issues with this patchset that I did not realize when I wrote it. Mainly related to the use of pseudo elements as checkmarks. Pseudo elements are semantically meaningless. From what I understand pseudo elements must only be used for decorative purposes only This would be an accessibility issue so I am failing QA on this until a better solution can be devised I notice the Bootstrap Collapse component uses pseudo elements but adds the "aria-expanded" attribute: https://getbootstrap.com/docs/3.4/javascript/#collapse Would that solve the issue? Might be nice to revisit this one :) Created attachment 192334 [details] [review] Bug 25314: Make OPAC facets collapse This patch implements collapsing facets using the Bootstrap Collapse component. To test, apply the patch and rebuild the OPAC CSS. 1. Clear your browser cache if necessary. 2. Using the sample data, go to Advanced Search in the OPAC. Check the "Books" checkbox under the "Item types" tab and submit. 3. In the left-hand sidebar you should see the facets you normally see, but now each one has an arrow icon to show whether the facet is expanded or collapsed. 4. All facets should be expanded by default. Adding expanded/collapsed settings to system preferences is out of scope for this bug. 5. Test expanding and collapsing facets, as well as the "Show more" link under each one. 6. Click the "Fiction" link under the "Collections" facet. When the refined search results load you clicked should now have a red "X" link next to "Fiction," which should no longer be a link. - Because "Fiction" is the only uption under the "Collections" facet, the facet is not collapsible. I reasoned that it would be important not to let a facet limitation disappear behind hidden facets. * The only way I know to do this in the template is to check for facets with only one option this has the possibly-unwanted side effect of making facets which haven't been selected but still only have one option not collapsible. In this example search "Holding libraries" and "Topics" show this. I'm open to suggestions on this! 7. Test with the displayFacetCount preference both enabled and disabled. Sponsored-by: Athens County Public Libraries Created attachment 192341 [details] [review] Bug 25314: Make OPAC facets collapse This patch implements collapsing facets using the Bootstrap Collapse component. To test, apply the patch and rebuild the OPAC CSS. 1. Clear your browser cache if necessary. 2. Using the sample data, go to Advanced Search in the OPAC. Check the "Books" checkbox under the "Item types" tab and submit. 3. In the left-hand sidebar you should see the facets you normally see, but now each one has an arrow icon to show whether the facet is expanded or collapsed. 4. All facets should be expanded by default. Adding expanded/collapsed settings to system preferences is out of scope for this bug. 5. Test expanding and collapsing facets, as well as the "Show more" link under each one. 6. Click the "Fiction" link under the "Collections" facet. When the refined search results load you clicked should now have a red "X" link next to "Fiction," which should no longer be a link. - Because "Fiction" is the only uption under the "Collections" facet, the facet is not collapsible. I reasoned that it would be important not to let a facet limitation disappear behind hidden facets. * The only way I know to do this in the template is to check for facets with only one option this has the possibly-unwanted side effect of making facets which haven't been selected but still only have one option not collapsible. In this example search "Holding libraries" and "Topics" show this. I'm open to suggestions on this! 7. Test with the displayFacetCount preference both enabled and disabled. Sponsored-by: Athens County Public Libraries Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Awesome, I know many libraries will like this change a lot. Created attachment 192597 [details]
Screenshot of facets with custom search filter
I quite like the changes, but wonder if we could make the custom search filters match in styling?
the [x] is glued to the word and doesn't present in the same style as the facets.
It's also missing a "tool tip" when hovering (might be a little out of scope, just noting).
Ran yarn build and viewed in a new private window - so hopefully CSS was correctly applied. Created attachment 192615 [details] [review] Bug 25314: (follow-up) Fix style of custom search filter clear button Created attachment 193046 [details] [review] y# Attachment to Bug 25314 - Make OPAC facets collapse Bug 25314: Make OPAC facets collapse This patch implements collapsing facets using the Bootstrap Collapse component. To test, apply the patch and rebuild the OPAC CSS. 1. Clear your browser cache if necessary. 2. Using the sample data, go to Advanced Search in the OPAC. Check the "Books" checkbox under the "Item types" tab and submit. 3. In the left-hand sidebar you should see the facets you normally see, but now each one has an arrow icon to show whether the facet is expanded or collapsed. 4. All facets should be expanded by default. Adding expanded/collapsed settings to system preferences is out of scope for this bug. 5. Test expanding and collapsing facets, as well as the "Show more" link under each one. 6. Click the "Fiction" link under the "Collections" facet. When the refined search results load you clicked should now have a red "X" link next to "Fiction," which should no longer be a link. - Because "Fiction" is the only uption under the "Collections" facet, the facet is not collapsible. I reasoned that it would be important not to let a facet limitation disappear behind hidden facets. * The only way I know to do this in the template is to check for facets with only one option this has the possibly-unwanted side effect of making facets which haven't been selected but still only have one option not collapsible. In this example search "Holding libraries" and "Topics" show this. I'm open to suggestions on this! 7. Test with the displayFacetCount preference both enabled and disabled. Sponsored-by: Athens County Public Libraries Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Created attachment 193047 [details] [review] Bug 25314: (follow-up) Fix style of custom search filter clear button Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Created attachment 193048 [details] [review] Bug 25314: Make OPAC facets collapse This patch implements collapsing facets using the Bootstrap Collapse component. To test, apply the patch and rebuild the OPAC CSS. 1. Clear your browser cache if necessary. 2. Using the sample data, go to Advanced Search in the OPAC. Check the "Books" checkbox under the "Item types" tab and submit. 3. In the left-hand sidebar you should see the facets you normally see, but now each one has an arrow icon to show whether the facet is expanded or collapsed. 4. All facets should be expanded by default. Adding expanded/collapsed settings to system preferences is out of scope for this bug. 5. Test expanding and collapsing facets, as well as the "Show more" link under each one. 6. Click the "Fiction" link under the "Collections" facet. When the refined search results load you clicked should now have a red "X" link next to "Fiction," which should no longer be a link. - Because "Fiction" is the only uption under the "Collections" facet, the facet is not collapsible. I reasoned that it would be important not to let a facet limitation disappear behind hidden facets. * The only way I know to do this in the template is to check for facets with only one option this has the possibly-unwanted side effect of making facets which haven't been selected but still only have one option not collapsible. In this example search "Holding libraries" and "Topics" show this. I'm open to suggestions on this! 7. Test with the displayFacetCount preference both enabled and disabled. Sponsored-by: Athens County Public Libraries Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Created attachment 193049 [details] [review] Bug 25314: (follow-up) Fix style of custom search filter clear button Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Thanks for the speedy follow-up, Owen! |