Steps to reproduce:

  1. Navigate to /admin/modules page
  2. Type something in the filter input to filter out modules
  3. Click × button to reset filters

Comments

Chi created an issue. See original summary.

chi’s picture

StatusFileSize
new1023 bytes

Not setting NR as the patch works only in Chrome. We probably need to change the event type to input to get it working correctly in all browsers.

kostyashupenko’s picture

Status: Active » Needs review
StatusFileSize
new497 bytes
djsagar’s picture

Status: Needs review » Reviewed & tested by the community
StatusFileSize
new2.14 MB

I applied patch #3 and it's working as accepted so we can move this as RTBC.
For reference, sharing video.

Thanks!

kostyashupenko’s picture

Status: Reviewed & tested by the community » Needs work

Here 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

kostyashupenko’s picture

Title: Module filter does not clear results on cancel click » Clicking X in input type search on module page on chrome does nothing
Status: Needs work » Needs review
Issue tags: +browser compatibility, +Needs tests, +Needs testing

Updating 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

marcoliver’s picture

Just to repeat my last comment from the duplicate issue. bnjmnm had pointed to the input event 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 input event.

So moving to the input event would break compatibility with that browser.

My proposed solution was to instead add a click event listener (which is of course not ideal, but should work across the board).

The last submitted patch, 2: module-filter-3371844-2.patch, failed testing. View results

lauriii’s picture

The 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.

marcoliver’s picture

Ah, thanks for the info, lauriii!

smustgrave credited bnjmnm.

smustgrave’s picture

Status: Needs review » Needs work

Added credit from closed duplicate issue

Moving to NW for the tests

bnjmnm’s picture

Sorry 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 keyup event was chosen when this was initially implemented many years ago, but support for input has since caught up. Glad to see core catching up too 😎

narendrar’s picture

Tested 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.js code to use the solution mentioned here in this issue.

narendrar’s picture

Status: Needs work » Needs review

Marking it as NR as manual testing is already done in #15. Not sure if anything is missing here.

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs testing

Not sure if this needs a test case but manual testing appears complete per #16.

Going to mark

The last submitted patch, 2: module-filter-3371844-2.patch, failed testing. View results

The last submitted patch, 2: module-filter-3371844-2.patch, failed testing. View results

narendrar’s picture

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 3: 3371844-3.patch, failed testing. View results

yash.rode’s picture

Version: 10.1.x-dev » 11.x-dev
StatusFileSize
new497 bytes
yash.rode’s picture

Status: Needs work » Needs review

I tested with the patch on chrome and safari, it is working as expected.

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Going to remark as failure appeared random. @yash.rode is there a change in #22 that's missing? #3 still applies cleanly to 11.x.

  • nod_ committed c9ced83e on 11.x
    Issue #3371844 by Chi, kostyashupenko, yash.rode, djsagar, narendraR,...

  • nod_ committed 76034628 on 10.2.x
    Issue #3371844 by Chi, kostyashupenko, yash.rode, djsagar, narendraR,...
nod_’s picture

Status: Reviewed & tested by the community » Fixed
Issue tags: -Needs tests

Committed c9ced83 and pushed to 11.x. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.