Bug 33381

Summary: Active link in the menu is not always correctly styled
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: ERMAssignee: Pedro Amorim <pedro.amorim>
Status: CLOSED FIXED QA Contact:
Severity: normal    
Priority: P5 - low CC: jonathan.druart, jonathan.field, lucas, martin.renvoize, pedro.amorim
Version: unspecified   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34691
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.05.00,22.11.05
Circulation function:
Bug Depends on:    
Bug Blocks: 33391    
Attachments: Bug 33381: Correct styling in the ERM menu
Bug 33381: Correct styling in the ERM menu
Bug 33381: Change router link active class match rule
Bug 33381: Fix sidemenu active and hover styling
Bug 33381: Fix sidemenu active and hover styling
Bug 33381: Fix sidemenu active and hover styling
Bug 33381: Fix sidemenu active and hover styling

Description Jonathan Druart 2023-03-31 09:49:05 UTC
If you are on /erm/agreements you will notice that the "Agreements" tab on the menu is correctly highlighted. However if you are on /erm/agreements/add it won't.
Comment 1 Jonathan Druart 2023-03-31 09:51:13 UTC
Created attachment 149022 [details] [review]
Bug 33381: Correct styling in the ERM menu

If you are on /erm/agreements you will notice that the "Agreements" tab on the
menu is correctly highlighted. However if you are on /erm/agreements/add it won't.

Test plan:
Go to /cgi-bin/koha/erm/agreements/add and confirm that the "Agreements"
tab in styled the same as when you are on /erm/agreements
Comment 2 Pedro Amorim 2023-04-01 14:15:22 UTC
Created attachment 149061 [details] [review]
Bug 33381: Correct styling in the ERM menu

If you are on /erm/agreements you will notice that the "Agreements" tab on the
menu is correctly highlighted. However if you are on /erm/agreements/add it won't.

Test plan:
Go to /cgi-bin/koha/erm/agreements/add and confirm that the "Agreements"
tab in styled the same as when you are on /erm/agreements

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 3 Pedro Amorim 2023-04-01 15:00:20 UTC
Hmm... Giving this a second look, I don't think we should be adding niche ERM styling cases onto staff-global.scss.

@Joubu this patch also "fixed" (unintentionally, I believe) the jumpy behavior I mentioned to you on hackfest, I opened bug 33391 for that.

As for this specific bug, I think the selector in your patch is wrong, it should be not(.disabled) instead of not(:disabled) because we're applying the "disabled" class, instead of using the "disabled" attribute.

I think we should either:
1) Fix this through styling but keep it in ERM files
2) Fix it through code (make sure .current class is added to the left menu item when in either FormAdd or Show views for that content type.
Comment 4 Pedro Amorim 2023-04-01 17:39:00 UTC
Created attachment 149063 [details] [review]
Bug 33381: Change router link active class match rule

This makes it so that the route erm/agreements/1 will also match the router-link /erm/agreements instead of the previous exact match rule, thus adding the 'current' class
Comment 5 Jonathan Druart 2023-04-04 07:27:02 UTC
(In reply to Pedro Amorim from comment #4)
> Created attachment 149063 [details] [review] [review]
> Bug 33381: Change router link active class match rule
> 
> This makes it so that the route erm/agreements/1 will also match the
> router-link /erm/agreements instead of the previous exact match rule, thus
> adding the 'current' class

Yes, I've tried that, but then you have other UI changes that are not expected:

https://snipboard.io/kc8Y32.jpg
Comment 6 Pedro Amorim 2023-04-05 08:34:25 UTC
Created attachment 149188 [details] [review]
Bug 33381: Fix sidemenu active and hover styling

Change router link active class match rule. This makes it so that the route erm/agreements/1 will also match the router-link /erm/agreements instead of the previous exact match rule, thus adding the 'current' class
Update styling router-links to not apply 'current' styling when
.disabled
Comment 7 Pedro Amorim 2023-04-05 09:07:16 UTC
Created attachment 149189 [details] [review]
Bug 33381: Fix sidemenu active and hover styling

Change router link active class match rule. This makes it so that the route erm/agreements/1 will also match the router-link /erm/agreements instead of the previous exact match rule, thus adding the 'current' class
Update styling router-links to not apply 'current' styling when
.disabled
Comment 8 Pedro Amorim 2023-04-05 09:10:56 UTC
Created attachment 149190 [details] [review]
Bug 33381: Fix sidemenu active and hover styling

Change router link active class match rule. This makes it so that the route erm/agreements/1 will also match the router-link /erm/agreements instead of the previous exact match rule, thus adding the 'current' class
Update styling router-links to not apply 'current' styling when
.disabled
Comment 9 Jonathan Druart 2023-04-06 07:21:57 UTC
Created attachment 149225 [details] [review]
Bug 33381: Fix sidemenu active and hover styling

Change router link active class match rule. This makes it so that the route erm/agreements/1 will also match the router-link /erm/agreements instead of the previous exact match rule, thus adding the 'current' class
Update styling router-links to not apply 'current' styling when
.disabled

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 10 Tomás Cohen Arazi (tcohen) 2023-04-06 12:30:43 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 11 Jacob O'Mara 2023-04-16 17:24:49 UTC
Nice work, thanks everyone!

Pushed to 22.11.x for the next release.