IBX-10608: Added method to reset checkbox states after actions in notifications list. #1751
Conversation
…ter actions in admin notifications list.
ezrobot
requested review from
GrabowskiM,
OstafinL,
albozek,
alekmick,
dew326 and
tischsoic
and removed request for
a team
src/bundle/Resources/public/js/scripts/admin.notifications.list.js
Outdated
Show resolved
Hide resolved
src/bundle/Resources/public/js/scripts/admin.notifications.list.js
Outdated
Show resolved
Hide resolved
…eckbox state persistence.
src/bundle/Resources/public/js/scripts/admin.notifications.list.js
Outdated
Show resolved
Hide resolved
| .filter((checkbox) => checkbox.checked) | ||
| .map((checkbox) => checkbox.dataset.notificationId); | ||
|
|
||
| if (!selectedNotifications.length) { |
There was a problem hiding this comment.
I wonder whether this check is really needed? Because "mark as read" button should be disabled if nothing is selected, so if this condition is false this function should not be called in the first place.
There was a problem hiding this comment.
Hmm, you have a point with the disable state of the button that I was not aware of.
In this specific case it might not be neccesarry then as this is just a double-check
| }); | ||
| }; | ||
|
|
||
| const clearCheckboxes = () => { |
There was a problem hiding this comment.
Maybe it would be better to place this logic in admin-ui/src/bundle/Resources/public/js/scripts/admin.table.js? And trigger it with an event. In this file there is a logic related to managing these selection checkboxes in all tables.
There was a problem hiding this comment.
I see the file now.
I think it would make more sense to have the logic there too
…check-checkboxes` event.
|
4 participants
Description:
Added
clearCheckboxesmethod to reset checkbox states after actions in admin notifications list.It worked fine in Chrome, but these changes are meant to fix the bug inside Firefox.
For QA:
Documentation: