Problem/Motivation

An optimization was noticed, in the issue about adding search to tests lists. #1919470-6: Add a search field to the test overview page

Proposed resolution

Replicate optimization for the modules search, which was added in #1848064: Allow to filter modules by arbitrary search strings on the Modules page.

Remaining tasks

  • Discuss if makes sense on the modules search.
  • Implement.

User interface changes

No UI changes.

API changes

No API changes.

Original report by @tim.plunkett and @berdir

Follow up for #1919470-6: Add a search field to the test overview page and #7

+++ b/core/modules/simpletest/simpletest.jsundefined
@@ -99,4 +99,62 @@ Drupal.behaviors.simpleTestSelectAll = {
+    var searching = false;
...
+        searching = true;
...
+      else if (searching) {

@tim.plunkett

This seems like a performance improvement that could be mimicked in the module filtering.

@Berdir

I'm not sure the modules search needs this. Unlike this, searching already starts at the second characters, so there's only a single character on which we would not have to search on. And there are also considerably more tests than modules (close to 900 table rows..)

But I'm happy to apply it there as well if this is the correct approach to do something like this.

CommentFileSizeAuthor
#13 core-js-filter-opti-1925492-13.patch1.14 KBvisabhishek
#9 core-js-filter-opti-1925492-9.patch1.65 KBnod_
#1 drupal-module-filter-optimization-1925492-1.patch1.09 KBmkadin

Comments

mkadin’s picture

mkadin commented
Status: Active » Needs review
StatusFileSize
new drupal-module-filter-optimization-1925492-1.patch1.09 KB

I don't think its critically important, but I do think its proper to avoid 'showing' all of the already visible rows.

Patch was easy enough to create, so I did it before further discussion :)

parthipanramesh’s picture

parthipanramesh commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Assigned: Unassigned » nod_
Category: Feature request » Task
Status: Reviewed & tested by the community » Needs review
Issue tags: +JavaScript

I don't see the harm, but passing to nod_ for a quick check.

nod_’s picture

nod_
Primary language French
Location Lille
commented
Assigned: nod_ » Unassigned
Status: Needs review » Needs work

doesn't apply.

And as it was said I'm not sure either that's needed. But I guess why not.

martin107’s picture

martin107 commented

Just tried to apply the patch. Not sure how code base has changed since the 7th but the patch applied OK now!

martin107’s picture

martin107 commented
Status: Needs work » Needs review
berdir’s picture

berdir
Primary language German
Location Switzerland
commented

1: drupal-module-filter-optimization-1925492-1.patch queued for re-testing.

giammi’s picture

giammi commented
Status: Needs review » Reviewed & tested by the community

Applied the patch and tested the following:

- entering 1 character does not start the filtering of search results
- entering 2nd character, search results are filtered
- when no search results are found, then no rows are shown

nod_’s picture

nod_
Primary language French
Location Lille
commented
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new core-js-filter-opti-1925492-9.patch1.65 KB

This is a reroll because the indentation standard changed for JavaScript files. No need for commit credit because of this reroll.

nod_’s picture

nod_
Primary language French
Location Lille
commented
Status: Needs review » Reviewed & tested by the community

Small whitespace diiff with previous patch on function declaration. auto-reformat FTW.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/modules/system/system.modules.js
@@ -17,17 +17,18 @@
-      function hidePackageDetails(index, element) {
+      function hidePackageDetails (index, element) {
...
-      function filterModuleList(e) {
+      function filterModuleList (e) {
...
-        function showModuleRow(index, row) {
+        function showModuleRow (index, row) {

auto-reformat--

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

Or maybe https://drupal.org/node/172169 needs an update?

visabhishek’s picture

visabhishek commented
Status: Needs work » Needs review
StatusFileSize
new core-js-filter-opti-1925492-13.patch1.14 KB

patch reformatted as par #11.

droplet’s picture

droplet commented
Status: Needs review » Reviewed & tested by the community

I reviewed Both #13 & #9.Extra changes in #9 are following JavaScript Code Standard

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Oh man! I thought I had committed this ages ago. So sorry!

Committed and pushed to 8.x. Thanks! :)

  • Commit 7628a6c on 8.x by webchick: 
    Issue #1925492 by visabhishek, nod_, mkadin | YesCT: Small optimization...

Status: Fixed » Closed (fixed)

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