Summary: | When a record has no items click delete all does not need an alert | ||
---|---|---|---|
Product: | Koha | Reporter: | Nick Clemens (kidclamp) <nick> |
Component: | Cataloging | Assignee: | Nick Clemens (kidclamp) <nick> |
Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
Severity: | normal | ||
Priority: | P5 - low | CC: | fridolin.somers, jonathan.druart, kyle, m.de.rooy |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | Trivial patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: |
22.05.00,21.11.05
|
|
Circulation function: | |||
Bug Depends on: | 11821 | ||
Bug Blocks: | |||
Attachments: |
Bug 25251: Remove alert from disabled buttons
Bug 25251: Remove alert from disabled buttons Bug 25251: Remove alert from disabled buttons Bug 25251: (follow-up) Remove alertNoItems Bug 25251: Remove alert from disabled buttons Bug 25251: (follow-up) Remove alertNoItems |
Description
Nick Clemens (kidclamp)
2020-04-22 13:49:52 UTC
It's a feature, from: commit 5c1691a8013bd38d6bae8b00a8950e1417695176 Author: Owen Leonard <oleonard@myacpl.org> Date: Fri Feb 21 15:27:41 2014 -0500 Bug 11821: disable catalog edit menu items and provide explanations when actions cannot be done Biblio edit menu items which are disabled should trigger a tooltip on hover and an alert on click with explanations. This patch implements this for the relevent menu items. <oleonard> kidclamp: But seriously, I think removing the alert is fine. Voiceover in OSX reads the tooltip so I think we're okay there. The alert is still there, if we wanted to remove it. Created attachment 131669 [details] [review] Bug 25251: Remove alert from disabled buttons To test: 1 - Find a record with items in the staff interface 2 - Click edit 3 - Note 'Edit items in a batch, 'Delete items in a batch', and 'Delete all items' are grayed out 4 - Note hovering displays a note that there are no items 5 - Click on each anyway, an alert is generated 6 - Apply patch 7 - Reload page 8 - Confirm no more alert 9 - Confirm tooltip is displayed Nick what would you think of this: $("#batchedit-disabled,#batchdelete-disabled,#deleteallitems-disabled") .on("click",function(e){ - e.preventDefault(); - alertNoItems(); + e.stopPropagation(); }) .tooltip(); This would remove the alert, but it would also prevent the menu from closing when the user clicks on a disabled link. This makes sense to me because a user clicking on a disabled link probably means they're not paying attention. In that case having nothing happen is better than having something happen (even if it's just the menu closing). (In reply to Owen Leonard from comment #5) > Nick what would you think of this: > > $("#batchedit-disabled,#batchdelete-disabled,#deleteallitems-disabled") > .on("click",function(e){ > - e.preventDefault(); > - alertNoItems(); > + e.stopPropagation(); > }) > .tooltip(); > > This would remove the alert, but it would also prevent the menu from closing > when the user clicks on a disabled link. This makes sense to me because a > user clicking on a disabled link probably means they're not paying > attention. In that case having nothing happen is better than having > something happen (even if it's just the menu closing). I would like that :-) I wanted it, but didn't know how to attain it - would you like to add the followup? Created attachment 131674 [details] [review] Bug 25251: Remove alert from disabled buttons This patch removes the alert, but adds a stopPropagation call in order to not close the menu when the disabled buttons are clicked To test: 1 - Find a record with items in the staff interface 2 - Click edit 3 - Note 'Edit items in a batch, 'Delete items in a batch', and 'Delete all items' are grayed out 4 - Note hovering displays a note that there are no items 5 - Click on each anyway, an alert is generated, menu is closed 6 - Apply patch 7 - Reload page 8 - Click on each disabled option and confirm - there is no more alert - the menu does not close 9 - Confirm tooltip is displayed Created attachment 131675 [details] [review] Bug 25251: Remove alert from disabled buttons This patch removes the alert, but adds a stopPropagation call in order to not close the menu when the disabled buttons are clicked To test: 1 - Find a record with items in the staff interface 2 - Click edit 3 - Note 'Edit items in a batch, 'Delete items in a batch', and 'Delete all items' are grayed out 4 - Note hovering displays a note that there are no items 5 - Click on each anyway, an alert is generated, menu is closed 6 - Apply patch 7 - Reload page 8 - Click on each disabled option and confirm - there is no more alert - the menu does not close 9 - Confirm tooltip is displayed Signed-off-by: Owen Leonard <oleonard@myacpl.org> Shouldn't we tweak confirm_items_deletion a bit and remove alertNoItems? We wwill never reach it anymore. Created attachment 132012 [details] [review] Bug 25251: (follow-up) Remove alertNoItems We should not need this function any longer I left the else conditional just in case Created attachment 132029 [details] [review] Bug 25251: Remove alert from disabled buttons This patch removes the alert, but adds a stopPropagation call in order to not close the menu when the disabled buttons are clicked To test: 1 - Find a record with items in the staff interface 2 - Click edit 3 - Note 'Edit items in a batch, 'Delete items in a batch', and 'Delete all items' are grayed out 4 - Note hovering displays a note that there are no items 5 - Click on each anyway, an alert is generated, menu is closed 6 - Apply patch 7 - Reload page 8 - Click on each disabled option and confirm - there is no more alert - the menu does not close 9 - Confirm tooltip is displayed Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 132030 [details] [review] Bug 25251: (follow-up) Remove alertNoItems We should not need this function any longer I left the else conditional just in case Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Totally makes sens Pushed to master for 22.05, thanks to everybody involved [U+1F984] Pushed to 21.11.x for 21.11.05 |