Steps to reproduce:
- Navigate to /admin/modules page
- Type something in the filter input to filter out modules
- Click
×button to reset filters
| Comment | File | Size | Author |
|---|---|---|---|
| #22 | 3371844-22.patch | 497 bytes | yash.rode |
| #4 | After-patch.mp4 | 2.14 MB | djsagar |
| #3 | 3371844-3.patch | 497 bytes | kostyashupenko |
| modules-filter.mkv | 97.4 KB | chi |
Comments
Comment #2
chi commentedNot setting NR as the patch works only in Chrome. We probably need to change the event type to
inputto get it working correctly in all browsers.Comment #3
kostyashupenkoComment #4
djsagar commentedI applied patch #3 and it's working as accepted so we can move this as RTBC.
For reference, sharing video.
Thanks!
Comment #5
kostyashupenkoHere is feedback from duplicated issue
Also here is a list of people who worked on duplicated issue (to include their names in commit):
- narendrar
- marcoliver
- bnjmnm
Comment #6
kostyashupenkoUpdating title of the issue.
Double checked if `input` event is enough instead of keyup - seems it's working on all browsers. `keyup` event is not required anymore. But it needs tests in all browsers once again since #4 tests are not not really obvious (not clear which browser was tested, and if only one browser or not). So need to test typing/clearing results in all browsers now
Comment #7
marcoliverJust to repeat my last comment from the duplicate issue. bnjmnm had pointed to the
inputevent as a good alternative (which I would agree to in principle), were it not for browser compatibility.https://www.drupal.org/docs/system-requirements/browser-requirements lists Opera mini as a supported browser for Core.
https://caniuse.com/input-event states that Opera mini does not support the
inputevent.So moving to the
inputevent would break compatibility with that browser.My proposed solution was to instead add a
clickevent listener (which is of course not ideal, but should work across the board).Comment #9
lauriiiThe Can I Use Opera Mini section is generated using the "Extreme data saving" mode which is not supported by Drupal: https://www.drupal.org/node/3217671.
Comment #10
marcoliverAh, thanks for the info, lauriii!
Comment #13
smustgrave commentedAdded credit from closed duplicate issue
Moving to NW for the tests
Comment #14
bnjmnmSorry if there was any confusion in the other issue that may have slowed things down. I did want to point out that browser compatibility was likely why the
keyupevent was chosen when this was initially implemented many years ago, but support forinputhas since caught up. Glad to see core catching up too 😎Comment #15
narendrarTested manually in Chrome, Firefox and Safari and the functionality is working as expected.
One thing to add, similar problem is fixed in https://www.drupal.org/project/drupal/issues/229193 with a different way. We can update
core/modules/user/user.permissions.jscode to use the solution mentioned here in this issue.Comment #16
narendrarMarking it as NR as manual testing is already done in #15. Not sure if anything is missing here.
Comment #17
smustgrave commentedNot sure if this needs a test case but manual testing appears complete per #16.
Going to mark
Comment #20
narendrarComment #22
yash.rode commentedComment #23
yash.rode commentedI tested with the patch on chrome and safari, it is working as expected.
Comment #24
smustgrave commentedGoing to remark as failure appeared random. @yash.rode is there a change in #22 that's missing? #3 still applies cleanly to 11.x.
Comment #27
nod_Committed c9ced83 and pushed to 11.x. Thanks!