I propose making facets collapse by defualt. I don't think we need to continue to have 'Show less' and 'Show more' buttons. If everything is collapsed by default we can show everything within that list when it is expanded.
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 :)