Description
Jonathan Druart
2023-03-31 09:49:05 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 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> 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. 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 (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 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 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 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 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> Pushed to master for 23.05. Nice work everyone, thanks! Nice work, thanks everyone! Pushed to 22.11.x for the next release. |