Bug 33381 - Active link in the menu is not always correctly styled
Summary: Active link in the menu is not always correctly styled
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: ERM (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal
Assignee: Pedro Amorim
QA Contact:
URL:
Keywords:
Depends on:
Blocks: 33391
  Show dependency treegraph
 
Reported: 2023-03-31 09:49 UTC by Jonathan Druart
Modified: 2024-07-04 20:37 UTC (History)
5 users (show)

See Also:
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:


Attachments
Bug 33381: Correct styling in the ERM menu (1.23 KB, patch)
2023-03-31 09:51 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 33381: Correct styling in the ERM menu (1.29 KB, patch)
2023-04-01 14:15 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 33381: Change router link active class match rule (1007 bytes, patch)
2023-04-01 17:39 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 33381: Fix sidemenu active and hover styling (1.82 KB, patch)
2023-04-05 08:34 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 33381: Fix sidemenu active and hover styling (1.85 KB, patch)
2023-04-05 09:07 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 33381: Fix sidemenu active and hover styling (2.03 KB, patch)
2023-04-05 09:10 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 33381: Fix sidemenu active and hover styling (2.10 KB, patch)
2023-04-06 07:21 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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.