Bug 33158 - Use template wrapper for authorized values and item types administration tabs
Summary: Use template wrapper for authorized values and item types administration tabs
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor (vote)
Assignee: Owen Leonard
QA Contact: Katrin Fischer
URL:
Keywords:
Depends on: 32571
Blocks:
  Show dependency treegraph
 
Reported: 2023-03-07 15:26 UTC by Owen Leonard
Modified: 2023-12-28 20:47 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.05.00,22.11.07


Attachments
Bug 33158: Use template wrapper for authorized values and item types administration tabs (18.46 KB, patch)
2023-03-07 15:43 UTC, Owen Leonard
Details | Diff | Splinter Review
Screenshots to show buggy behaviour on testing for signoff (22.65 KB, image/png)
2023-03-31 15:36 UTC, Andrew Auld
Details
Screenshot2 to show buggy behaviour on testing for signoff (20.79 KB, image/png)
2023-03-31 15:37 UTC, Andrew Auld
Details
Bug 33158: Use template wrapper for authorized values and item types administration tabs (19.34 KB, patch)
2023-04-13 18:25 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 33158: Use template wrapper for authorized values and item types administration tabs (19.39 KB, patch)
2023-04-14 11:26 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 33158: (follow-up) Fix for QA test failure (944 bytes, patch)
2023-05-09 16:50 UTC, Owen Leonard
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Owen Leonard 2023-03-07 15:26:59 UTC
Continuing the work of Bug 32571, the authorized values administration page should be updated to use the new WRAPPER for tabs.
Comment 1 Owen Leonard 2023-03-07 15:43:43 UTC Comment hidden (obsolete)
Comment 2 Alexandra Speer 2023-03-28 13:41:01 UTC
Hello, 

everything works, except the following in Item types and Authorized values:
If you did not previously have any icon selected, the "None" tab should be active. There is no active Tab.
Comment 3 Owen Leonard 2023-03-29 14:11:28 UTC
(In reply to Alexandra Speer from comment #2)
> If you did not previously have any icon selected, the "None" tab should be
> active. There is no active Tab.

I believe this has just been fixed by Bug 33278
Comment 4 Andrew Auld 2023-03-31 15:35:31 UTC
Everything works correctly in item types, including for adding the remote image. However, when you go back to edit the item type with the remote image, I think there is a bug in the display. Initially it lands correctly with the 'remote image' tab active. However, as well as the radio button for the 'Remote image' and the field for the url there is also a 'None' radio button and label. If I switch between that active 'Remote image' tab and another, then back again, this 'None' radio button disappears.
See screenshots.

In authorised values, everything works correctly EXCEPT for the remote image option. It allows you to set a remote image, and correctly displays the remote image. However, when you go back to edit the authorised value, it starts on the "None" tab, unselected. When you then click on the 'Remote image' tab, the radio button is unselected and there is no url showing in the field. Saving out of that screen removes the remote image, even without selecting the 'None' radio button on the 'None' tab.
Comment 5 Andrew Auld 2023-03-31 15:36:43 UTC
Created attachment 149041 [details]
Screenshots to show buggy behaviour on testing for signoff
Comment 6 Andrew Auld 2023-03-31 15:37:10 UTC
Created attachment 149042 [details]
Screenshot2 to show buggy behaviour on testing for signoff
Comment 7 Andrew Auld 2023-03-31 15:39:56 UTC
(In reply to Andrew Auld from comment #4)
> Saving out of that screen removes the remote image, even without
> selecting the 'None' radio button on the 'None' tab.

NB cancelling out of the screen leaves the remote image in place.
Comment 8 Owen Leonard 2023-04-13 18:25:38 UTC Comment hidden (obsolete)
Comment 9 PTFS Europe Sandboxes 2023-04-14 11:26:48 UTC
Created attachment 149669 [details] [review]
Bug 33158: Use template wrapper for authorized values and item types administration tabs

This patch updates the authorized values and item types administration
templates so that they use the new WRAPPER directive to build tabbed
navigation.

Seeing that the markup in itemtypes.tt and authorised_values.tt is
indential when it comes to icon selection, I have moved that section of
the template into an include file and updated both templates to use it.

The patch also makes minor SCSS changes, so to test you must rebuild the
staff interface CSS.

To test, apply the patch and go to Administration -> Item types.

- Edit an item type.
- On the edit page you should see tabs under "Choose an icon."
- Confirm that the tabs look correct and work correctly.
- If you did not previously have any icon selected, the "None" tab
  should be active.
- If you had an icon selected, that icon set's tab should be active.
- Confirm that if you specify a remote image
  (e.g.https://via.placeholder.com/50/FF0000/FFFFFF.png) the tab is
  correctly shown after you save and re-edit.
- Confirm that changing icons works correctly and that the selected
  icon's tab is always active when you return to the edit view.

Perform the same tests in Administration -> Authorized values.

Signed-off-by: Andrew Auld <andrew.auld@ptfs-europe.com>
Comment 10 Andrew Auld 2023-04-14 11:27:16 UTC
Great! Works perfectly now.
Comment 11 Katrin Fischer 2023-04-20 20:49:01 UTC
Can you please check on the QA test fail?

 FAIL	koha-tmpl/intranet-tmpl/prog/en/includes/admin-icon-selection.inc
   FAIL	  valid_template
		tab_item: not found
Comment 12 Owen Leonard 2023-05-09 16:50:47 UTC
Created attachment 150893 [details] [review]
Bug 33158: (follow-up) Fix for QA test failure

The page seems to work fine without these changes but the QA test script
is happier with "USE raw" and html_helpers.inc added.
Comment 13 Tomás Cohen Arazi 2023-05-16 18:19:15 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 14 Pedro Amorim 2023-06-07 12:18:39 UTC
Nice work everyone!

Pushed to 22.11.x for next release