|
Description
Owen Leonard
2023-04-13 14:56:21 UTC
Created attachment 151313 [details] [review] Bug 33528: Use template wrapper for tabs: Patron details page This patch updates the checkout and patron details templates so that they use the new WRAPPER directive to build tabbed navigation. The markup for the tab navigation and tab panels is moved into its own include file since the templates were duplicating essentially the same code. To fully test you should have patrons with checkouts, fines, holds, recalls, and article requests. Apply the patch and go to the checkout page in the staff interface. With each tab, test as much functionality as you can within that tab. - Checkouts - Holds - Recalls (with UseRecalls enabled) - Claims (with a LOST value defined in ClaimReturnedLostValue) - Restrictions - Article Requests (with ArticleRequests preference enabled) - Clubs (with at least one club defined) - Relatives' checkouts (patron must have a guarantee linked to their account). Perform the same tests on the patron details page. Note that the "Charges" tab is shown on the patron details page but not the checkout page. This is not a change made by this patch. Not expecting this to go into 23.05 but I thought I'd submit it in case someone can start testing it. *** Bug 33527 has been marked as a duplicate of this bug. *** Created attachment 151887 [details] [review] Bug 33528: Use template wrapper for tabs: Patron details page This patch updates the checkout and patron details templates so that they use the new WRAPPER directive to build tabbed navigation. The markup for the tab navigation and tab panels is moved into its own include file since the templates were duplicating essentially the same code. To fully test you should have patrons with checkouts, fines, holds, recalls, and article requests. Apply the patch and go to the checkout page in the staff interface. With each tab, test as much functionality as you can within that tab. - Checkouts - Holds - Recalls (with UseRecalls enabled) - Claims (with a LOST value defined in ClaimReturnedLostValue) - Restrictions - Article Requests (with ArticleRequests preference enabled) - Clubs (with at least one club defined) - Relatives' checkouts (patron must have a guarantee linked to their account). Perform the same tests on the patron details page. Note that the "Charges" tab is shown on the patron details page but not the checkout page. This is not a change made by this patch. I ran into some problems testing the club tab in both the check out page and the details page. The club shows up fine, however after clicking the enroll button, the "finish enrollment" button appears on the screen visually plain text rather than a button. And after clicking on the "finish enrollment" text, the club tab essentially disappears but all of the information from the tab is left on the screen. From here if you try to click on another tab, the formatting really gets messed up and the table turns green for some reason. Refreshing the page did put the club tab back in the correct spot and also fixed the formatting, but if you try to cancel your enrollment in the club, a pop up appears that says: "Something went wrong when loading the table." After closing the popup, the formatting of the club tab gets messed up again, with the same problems as described before. (In reply to Sam Lau from comment #5) > I ran into some problems testing the club tab in both the check out page and > the details page. I am not seeing this in my test system. Could you try again? Are there any JavaScript errors in the console? (In reply to Owen Leonard from comment #6) > (In reply to Sam Lau from comment #5) > > I ran into some problems testing the club tab in both the check out page and > > the details page. > > I am not seeing this in my test system. Could you try again? Are there any > JavaScript errors in the console? I also recreate this. Steps I used: 1. Apply patch 2. Go to patron checkout or detail tab 3. Click on Clubs tab > Enroll 4. Above the table click 'Finish enrollment' or 'Cancel' 5. Clubs panel now appears in unusual position. 6. Click on another tab (Checkouts, Holds ) and the Clubs panel now has a green background. Created attachment 151935 [details]
Screenshot of display
Here is a video of the bug as well: https://www.youtube.com/watch?v=bcNWwtvDuVc Created attachment 151980 [details] [review] Bug 33528: Use template wrapper for tabs: Patron details page This patch updates the checkout and patron details templates so that they use the new WRAPPER directive to build tabbed navigation. The markup for the tab navigation and tab panels is moved into its own include file since the templates were duplicating essentially the same code. To fully test you should have patrons with checkouts, fines, holds, recalls, and article requests. Apply the patch and go to the checkout page in the staff interface. With each tab, test as much functionality as you can within that tab. - Checkouts - Holds - Recalls (with UseRecalls enabled) - Claims (with a LOST value defined in ClaimReturnedLostValue) - Restrictions - Article Requests (with ArticleRequests preference enabled) - Clubs (with at least one club defined) - Relatives' checkouts (patron must have a guarantee linked to their account). Perform the same tests on the patron details page. Note that the "Charges" tab is shown on the patron details page but not the checkout page. This is not a change made by this patch. (In reply to Sam Lau from comment #9) > Here is a video of the bug as well Thanks, I'm not sure why I wasn't seeing it before I'd failed to update all the places where the changed DOM structure needed to be updated in the JavaScript Created attachment 151984 [details] [review] Bug 33528: Use template wrapper for tabs: Patron details page This patch updates the checkout and patron details templates so that they use the new WRAPPER directive to build tabbed navigation. The markup for the tab navigation and tab panels is moved into its own include file since the templates were duplicating essentially the same code. To fully test you should have patrons with checkouts, fines, holds, recalls, and article requests. Apply the patch and go to the checkout page in the staff interface. With each tab, test as much functionality as you can within that tab. - Checkouts - Holds - Recalls (with UseRecalls enabled) - Claims (with a LOST value defined in ClaimReturnedLostValue) - Restrictions - Article Requests (with ArticleRequests preference enabled) - Clubs (with at least one club defined) - Relatives' checkouts (patron must have a guarantee linked to their account). Perform the same tests on the patron details page. Note that the "Charges" tab is shown on the patron details page but not the checkout page. This is not a change made by this patch. Signed-off-by: Sam Lau <samalau@gmail.com> First time I noticed this: when doing actions on the tabs, the page reloads and you lose your 'active' tab. Maybe we could take a look at this sometime. But this is the same without the patch set. Created attachment 152216 [details] [review] Bug 33528: Use template wrapper for tabs: Patron details page This patch updates the checkout and patron details templates so that they use the new WRAPPER directive to build tabbed navigation. The markup for the tab navigation and tab panels is moved into its own include file since the templates were duplicating essentially the same code. To fully test you should have patrons with checkouts, fines, holds, recalls, and article requests. Apply the patch and go to the checkout page in the staff interface. With each tab, test as much functionality as you can within that tab. - Checkouts - Holds - Recalls (with UseRecalls enabled) - Claims (with a LOST value defined in ClaimReturnedLostValue) - Restrictions - Article Requests (with ArticleRequests preference enabled) - Clubs (with at least one club defined) - Relatives' checkouts (patron must have a guarantee linked to their account). Perform the same tests on the patron details page. Note that the "Charges" tab is shown on the patron details page but not the checkout page. This is not a change made by this patch. Signed-off-by: Sam Lau <samalau@gmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 152291 [details] [review] Bug 33528: Use template wrapper for tabs: Patron details page This patch updates the checkout and patron details templates so that they use the new WRAPPER directive to build tabbed navigation. The markup for the tab navigation and tab panels is moved into its own include file since the templates were duplicating essentially the same code. To fully test you should have patrons with checkouts, fines, holds, recalls, and article requests. Apply the patch and go to the checkout page in the staff interface. With each tab, test as much functionality as you can within that tab. - Checkouts - Holds - Recalls (with UseRecalls enabled) - Claims (with a LOST value defined in ClaimReturnedLostValue) - Restrictions - Article Requests (with ArticleRequests preference enabled) - Clubs (with at least one club defined) - Relatives' checkouts (patron must have a guarantee linked to their account). Perform the same tests on the patron details page. Note that the "Charges" tab is shown on the patron details page but not the checkout page. This is not a change made by this patch. Signed-off-by: Sam Lau <samalau@gmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 152292 [details] [review] Bug 33528: (follow-up) Correct selector for event handler The event handler for restriction deletion needs to be adjusted according to the new container ID. Without this change the JavaScript confirmation message didn't appear and restrictions were deleted without confirmation. Pushed to master for 23.11. Nice work everyone, thanks! Created attachment 152344 [details] [review] Bug 33528: Fix basic_workflow.t Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Thanks for all the hard work! Pushed to 23.05.x for the next release This was a fairly involved backport, but I felt it made sense to aid future backports and conflicts there. Does not apply to 22.11.x. Not pushing. |