Bug 39189 - Collapsing sections on moredetail.pl not working
Summary: Collapsing sections on moredetail.pl not working
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Owen Leonard
QA Contact: Lisette Scheer
URL:
Keywords:
Depends on: 7508
Blocks:
  Show dependency treegraph
 
Reported: 2025-02-24 12:35 UTC by Owen Leonard
Modified: 2025-03-17 07:28 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
25.05.00
Circulation function:


Attachments
Bug 39189: Fix collapsing panels on moredetail.tt (3.52 KB, patch)
2025-02-24 13:06 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 39189: Fix collapsing panels on moredetail.tt (3.52 KB, patch)
2025-02-28 16:05 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 39189: (follow-up) Add containers to the template markup (35.41 KB, patch)
2025-02-28 16:05 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 39189: (follow-up) Add a missing container to patron entry form (11.43 KB, patch)
2025-03-11 11:18 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 39189: (follow-up) Add missing container to moredetail.tt (58.12 KB, patch)
2025-03-14 11:45 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 39189: Fix collapsing panels on moredetail.tt (3.57 KB, patch)
2025-03-14 18:35 UTC, Phil Ringnalda
Details | Diff | Splinter Review
Bug 39189: (follow-up) Add containers to the template markup (35.46 KB, patch)
2025-03-14 18:35 UTC, Phil Ringnalda
Details | Diff | Splinter Review
Bug 39189: (follow-up) Add a missing container to patron entry form (11.49 KB, patch)
2025-03-14 18:35 UTC, Phil Ringnalda
Details | Diff | Splinter Review
Bug 39189: (follow-up) Add missing container to moredetail.tt (58.18 KB, patch)
2025-03-14 18:35 UTC, Phil Ringnalda
Details | Diff | Splinter Review
Bug 39189: Fix collapsing panels on moredetail.tt (3.63 KB, patch)
2025-03-14 20:33 UTC, Lisette Scheer
Details | Diff | Splinter Review
Bug 39189: (follow-up) Add containers to the template markup (35.52 KB, patch)
2025-03-14 20:34 UTC, Lisette Scheer
Details | Diff | Splinter Review
Bug 39189: (follow-up) Add a missing container to patron entry form (11.55 KB, patch)
2025-03-14 20:34 UTC, Lisette Scheer
Details | Diff | Splinter Review
Bug 39189: (follow-up) Add missing container to moredetail.tt (58.24 KB, patch)
2025-03-14 20:34 UTC, Lisette Scheer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Owen Leonard 2025-02-24 12:35:32 UTC
I should have reworked Bug 7508 on top of Bug 38227, because 38227 moved the togglePanel function to staff-global.js. As it is now, there is a duplicate togglePanel function in moredetail.tt and the panels are not working correctly.
Comment 1 Owen Leonard 2025-02-24 12:43:36 UTC
(In reply to Owen Leonard from comment #0)
> a duplicate togglePanel function in moredetail.tt

Sorry, a duplicate click handler.
Comment 2 Owen Leonard 2025-02-24 13:06:37 UTC
Created attachment 178551 [details] [review]
Bug 39189: Fix collapsing panels on moredetail.tt

This patch removes a duplicate togglePanel function and a duplicate
click handler from moredetail.tt. Bug 38227 made both global by adding
them to staff-global.js.

Also fixed: Converted a mistaken use of "__" to "_".

To test, apply the patch and clear your browser cache if necessary.

- Perform a catalog search and view the details of a title with multiple
  items.
- On the bibliographic detail page, click "Items" in the sidebar.
- Click a "Barcode XXXX" heading to confirm that it collapses and
  expands that section correctly.
- Collapse all sections and then test the barcode links in the sidebar.
  Clicking any of them should expand the correct panel and scroll to
  that section.
- Test the other pages using the togglePanel function: Patron entry and
  the authority search from triggered from the MARC editor.
Comment 3 Magnus Enger 2025-02-28 08:27:20 UTC
Sorry, doesn't quite work for me. 

Applied the patch
yarn js:build
restart_all

Did a search, clicked through to the details of a record that has three items, and then on "Items" in the sidebar. 

I see the barcodes under "Items" in the sidebar. Clicking on them scrolls the page so the heading for the chosen item is displayed at the top of the screen. Clickin on the heading of that item does nothing. Scrolling to another heading and clicking on that brings the heading to the top of the screen, but it is never collapsed. There is nothing in the console.
Comment 4 Owen Leonard 2025-02-28 14:30:54 UTC
(In reply to Magnus Enger from comment #3)
> Clickin on the heading of that item does nothing. Scrolling to
> another heading and clicking on that brings the heading to the top of the
> screen, but it is never collapsed. There is nothing in the console.

I would expect this behavior if the browser was still using cached JS, but I know you did a hard refresh on the page so I'm not sure what's going on. I tested it again in a few different browsers and it still works for me.

Perhaps someone else could try, maybe they'll have better luck?
Comment 5 Brendan Lawlor 2025-02-28 15:39:34 UTC
When I tested this it worked as expected for the more details page.

For this step: 
Test the other pages using the togglePanel function 

All the sections in Patron entry collapsed and expanded correctly except for:
Additional attributes and identifiers
Patron messaging preferences

Clicking on those headings didn't do anything. Maybe that's a separate bug?

For this step:
  the authority search from triggered from the MARC editor.

I couldn't test this. What I thought is Authority search doesn't have any collapsible panels, but I can look again if you can add some info on how to do this part.
Comment 6 Owen Leonard 2025-02-28 16:05:27 UTC
Created attachment 178865 [details] [review]
Bug 39189: Fix collapsing panels on moredetail.tt

This patch removes a duplicate togglePanel function and a duplicate
click handler from moredetail.tt. Bug 38227 made both global by adding
them to staff-global.js.

Also fixed: Converted a mistaken use of "__" to "_".

To test, apply the patch and clear your browser cache if necessary.

- Perform a catalog search and view the details of a title with multiple
  items.
- On the bibliographic detail page, click "Items" in the sidebar.
- Click a "Barcode XXXX" heading to confirm that it collapses and
  expands that section correctly.
- Collapse all sections and then test the barcode links in the sidebar.
  Clicking any of them should expand the correct panel and scroll to
  that section.
- Test the other pages using the togglePanel function: Patron entry and
  the authority search from triggered from the MARC editor.
Comment 7 Owen Leonard 2025-02-28 16:05:37 UTC
Created attachment 178866 [details] [review]
Bug 39189: (follow-up) Add containers to the template markup

Some sections of the patron entry form do not have a single container to
be collapsed using the jQuery's next(). Since this patch converts it
from nextAll() it's necessary to have a single container to match.
Comment 8 Phil Ringnalda 2025-03-01 01:12:13 UTC
(In reply to Brendan Lawlor from comment #5)
> For this step:
>   the authority search from triggered from the MARC editor.
> 
> I couldn't test this. What I thought is Authority search doesn't have any
> collapsible panels, but I can look again if you can add some info on how to
> do this part.

For an authority controlled tag like 100 click the "Tag editor" at the end of subfield a. In the popup, you can collapse the "Search options" panel that contains the form. Do a search that returns one or more results, at the top is a collapsed "Search options" panel that expands to show the search form. Do a search that returns no results, the panel will be expanded by default, but you can collapse it to concentrate on the "No results found" message.
Comment 9 Phil Ringnalda 2025-03-06 23:39:34 UTC
Everything else seems to work, but on the patron entry form collapsing "Patron account flags" collapses the paragraph of text, but leaves the Gone no address and Lost card radiobuttons uncollapsed.
Comment 10 Owen Leonard 2025-03-11 11:18:18 UTC
Created attachment 179157 [details] [review]
Bug 39189: (follow-up) Add a missing container to patron entry form
Comment 11 Phil Ringnalda 2025-03-14 00:56:18 UTC
Works fine for the patron entry form now.

I must have been too focused on the links expanding and collapsing individual items, I missed seeing that apparently all items are children of the item above them.

Steps to reproduce:
1. Find a record with three items, like Empress of the blues
2. On the items tab, scroll down to the bottom item and collapse it
3. Now you have two expanded items, and one collapsed
4. Collapse the second item
5. Now you have one expanded item, and one collapsed item, rather than two
6. Collapse the first item
7. Now you have one collapsed item, rather than three
Comment 12 Owen Leonard 2025-03-14 11:45:16 UTC
Created attachment 179323 [details] [review]
Bug 39189: (follow-up) Add missing container to moredetail.tt
Comment 13 Phil Ringnalda 2025-03-14 18:35:38 UTC
Created attachment 179346 [details] [review]
Bug 39189: Fix collapsing panels on moredetail.tt

This patch removes a duplicate togglePanel function and a duplicate
click handler from moredetail.tt. Bug 38227 made both global by adding
them to staff-global.js.

Also fixed: Converted a mistaken use of "__" to "_".

To test, apply the patch and clear your browser cache if necessary.

- Perform a catalog search and view the details of a title with multiple
  items.
- On the bibliographic detail page, click "Items" in the sidebar.
- Click a "Barcode XXXX" heading to confirm that it collapses and
  expands that section correctly.
- Collapse all sections and then test the barcode links in the sidebar.
  Clicking any of them should expand the correct panel and scroll to
  that section.
- Test the other pages using the togglePanel function: Patron entry and
  the authority search from triggered from the MARC editor.

Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Comment 14 Phil Ringnalda 2025-03-14 18:35:41 UTC
Created attachment 179347 [details] [review]
Bug 39189: (follow-up) Add containers to the template markup

Some sections of the patron entry form do not have a single container to
be collapsed using the jQuery's next(). Since this patch converts it
from nextAll() it's necessary to have a single container to match.

Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Comment 15 Phil Ringnalda 2025-03-14 18:35:44 UTC
Created attachment 179348 [details] [review]
Bug 39189: (follow-up) Add a missing container to patron entry form

Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Comment 16 Phil Ringnalda 2025-03-14 18:35:47 UTC
Created attachment 179349 [details] [review]
Bug 39189: (follow-up) Add missing container to moredetail.tt

Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Comment 17 Lisette Scheer 2025-03-14 20:33:59 UTC
Created attachment 179352 [details] [review]
Bug 39189: Fix collapsing panels on moredetail.tt

This patch removes a duplicate togglePanel function and a duplicate
click handler from moredetail.tt. Bug 38227 made both global by adding
them to staff-global.js.

Also fixed: Converted a mistaken use of "__" to "_".

To test, apply the patch and clear your browser cache if necessary.

- Perform a catalog search and view the details of a title with multiple
  items.
- On the bibliographic detail page, click "Items" in the sidebar.
- Click a "Barcode XXXX" heading to confirm that it collapses and
  expands that section correctly.
- Collapse all sections and then test the barcode links in the sidebar.
  Clicking any of them should expand the correct panel and scroll to
  that section.
- Test the other pages using the togglePanel function: Patron entry and
  the authority search from triggered from the MARC editor.

Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Comment 18 Lisette Scheer 2025-03-14 20:34:01 UTC
Created attachment 179353 [details] [review]
Bug 39189: (follow-up) Add containers to the template markup

Some sections of the patron entry form do not have a single container to
be collapsed using the jQuery's next(). Since this patch converts it
from nextAll() it's necessary to have a single container to match.

Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Comment 19 Lisette Scheer 2025-03-14 20:34:04 UTC
Created attachment 179354 [details] [review]
Bug 39189: (follow-up) Add a missing container to patron entry form

Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Comment 20 Lisette Scheer 2025-03-14 20:34:06 UTC
Created attachment 179355 [details] [review]
Bug 39189: (follow-up) Add missing container to moredetail.tt

Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Comment 21 Katrin Fischer 2025-03-17 07:08:24 UTC
Especially happy to see accessibility improved too :)

(Removing "Click to ...")
Comment 22 Katrin Fischer 2025-03-17 07:28:27 UTC
Pushed for 25.05!

Well done everyone, thank you!