Create Javascript library for searching rendered lists on the client.

I'm having a go at dabbling with this. I think I can see how to follow the same sort of pattern as tabledrag, which allows specific uses of it to customise it.

There's still handling of table grouping to do, and then conversion of existing filtering to the library, but I'd really appreciate a sanity check of my totally amateur attempt at JavaScript :)

Found a bug when a DETAILS element starts off closed but an item is found in that element. The whole DETAILS gets hidden! I wonder if when the DETAILS is closed, the things inside it count as not visible?

The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Something to discuss: the current filtering implementations are not consistent in their behaviour, on two axes:

- filter on first character typed / filter only on a minimum of 2 characters typed
- search from the start of words / search inside words -- in other words, will typing 'om' find 'comment' or not?

1. Place block on Block layout - 2 chars, search in words
2. Views listing - 2 chars, search in words
3. Extend (modules listing) - 2 chars, search from start of words
4. Layout builder blocks - 2 chars search in words
5. Field reuse selection - 1 char, search in words

Do we want to standardize as part of this issue, or make the new library configurable for these behaviours?

@joachim: I'd suggest make it configurable.

FTR: Some time i hacked down livefilter → module, which exposes similar functionality as a block. Based on this library, we may move that block to core in a followup.

Notes to self, still to do:

- announce functionality
- fix details opening/closing - needs to remember state prior to filtering
- options to word boundaries & minimum search string
- set up options for all conversions to match current behaviour

I think this is ready for review.

I'm stuck on the last two complaints the JS linting has:

> error jquery/no-is Prefer matches to $.is https://git.drupalcode.org/issue/drupal-3028968/-/blob/4d82de62/core/mis...

I don't think we can use matches() here, as for the user permissions table, the selector for groups has to be "tr:has(td.someclass)" and in Firefox, that doesn't work with matches().

I don't know how to rewrite this to avoid using $.is.

I have a jQuery object that is all the groups, and a jQuery object that is the current TR from the table, and I need to determine whether the TR is one of the groups or not.

> fatal Parsing error: Unexpected token = https://git.drupalcode.org/issue/drupal-3028968/-/blob/4d82de62/core/mis...

This is because we don't have ??= in our JS standards yet. What's the alternative?

Gonna check js

Rebased the branch on 11.x. Also applied a couple of fixes that got made to core/modules/system/js/system.modules.js in the meantime to the code that replaces that file.

Added vanilla JS only library.

We need to check backend and tests.

@joachim @nod_
Actual version of that library in this MR https://www.drupal.org/project/navigation/issues/3412125#comment-15461104 📌 Convert jQuery to vanilla Javascript Needs work

We will continue with adaptation this library in Navigation module and then backport it to core as it was disqussed in slack `admin-ui`

How much more work does it need? I didn't think it needed to be extensively developed in contrib - could we just improve the code here?

RE #31
more details here:
https://drupal.slack.com/archives/C7AB68LJV/p1708627699667799

Last variant of this library in Navigation module was here

https://git.drupalcode.org/project/drupal/-/merge_requests/7474

It is removed now from navigation module since it uses Layout Builder for now.

to help with searchability in the queue

Some weird functional js errors. Cannot reproduce them locally. And it is mostly ready.

I suggest to postpone views popup filters cause it has extra selectbox filter.

@finnsky your MR seems to have dropped the ListFilter.php render element I'd added. I feel that having a dedicated render element gives clearer DX than just adding attributes. Also, it means that you get to choose where to position the search element.

Yeah. Thank you. I meant ready on front side. Can be any adaptation on backend.

Really not sure what the reason of failures. It should work. Probably filterVisibleElements() works incorrect.

joachim → changed the visibility of the branch 3028968-create-js-library-filtering-lists to hidden.

@andy
thank you for review.
gonna check now.
next time you also can add direct comment here. since gitlab only comment not updates issue status.

Fixed tests. Maybe not in good way.

Now ready for review in terms of frontend.

@joachim, please take a look if you want to adapt render elements to current backend.

Last weird test failure. I give up

> @joachim, please take a look if you want to adapt render elements to current backend.

I'm not sure when I'll have time.

I'm feeling a bit discouraged about this also, as it feels like doing the work I did all over again.

Can you tell me which properties of ListFilter are for functionality your JS supports, and which should be removed?

- #library for example can be removed, as you said in Slack that your JS handles sibling groups and details.

What about #minimum_filter_length and #search_start_of_words? I can't tell from reading drupal-filter.js whether it handles those -- the code could really do with more comments.

and one question. at the moment the filter field is using the browser based clear button. that has the downside that the behavior differs in between browsers and in firefox there is no clear button at all. would it make sense to include the suggestion from this issue ✨ Add a clear button to the fields ui Active , adding the clear button functionality recently added to project browser, already in this issue, or keep the scope tight and keep ✨ Add a clear button to the fields ui Active as a followup?

i've also manually tested the filter with a screenreader (voiceover in my case - see filter.mp4 - recorded the video a few days ago but managed to finish the write up just now). there are a few details to note:

somehow the announcement is sort of cut when the initial string is entered. in the first example entered ( "basic block" ) you can only hear "6 permissions", the rest of the string "are available in the modified list" gets cut by some announcement about the styling, and then just "block" gets announced again even though "basic block" was entered. when the already entered string "basic block" gets adjusted to "basic" the full string "15 permissions are available in the modified list." is getting announced. but "basic" is not announced after the announcement of "15 permissions are available in the modified list." like with the entry of "basic block" and the additional announcement of "block" before. BUT in the video when the filter is cleared by pressing the ESC key "basic" is announced before "all available permissions are listed". That is potentially confusing. And the pattern is reproducible if you enter arbitrary characters ("bvgfh"), again "0 permissions" is announced interrupted by some styling announcement, the rest of the string "are available in the modified list" is getting dropped. after that the arbitrary string is announced again. if you clear the filter field with ESC "bvgfh" is announced another time for the empty filter field followed by " All available permissions are listed"

In regards of the micro copy used, "modified list" in the context of the example "x permissions are available in the modified list." raises potential questions and make the user at least think. Me as the user could ask what does modified list mean in this context? Which list and what is "modified" implying; results added and or removed or even altered entries? Why not go with something like "x filter results"? That would be slightly more generic but shorter and more clear.

There is also a problem with WCAG 2.2. SC3.2.2 (https://www.w3.org/WAI/WCAG22/Understanding/on-input.html) in the current MR as well as in Core in a few places. Discussed that aspect with @mgifford. In short the goal of that success criterion is that the interface can be operated predictably by the user. At the moment as soon as the user is entering any character in the filter field the results are getting updated based on that entry immediately. The user doesn't have to press return, nor is there a submit button, nor any "warning" in the filter label. The content gets re-rendered immediately based on the entered filter string . The option might be to either add a filter button and wait with filtering until the filter button/enter is pressed or at least add some sort of "warning" to the label of the filter field so the user is prepared that the list will be filtered based on the entry immediately.

> in the current MR as well as in Core in a few places

Are there any existing filters in core which work OK with screen reader announcements? If so, the new script should take from those.

Hello, guys, do you want me to go over the code and try to remove jQuery, switching it to a pure Vanilla JS approach?

On this ticket 📌 Improve the performance of the script delivering the permissions search filter functionality in admin/people/permissions. Needs review I was able to achieve an excellent performance enhancement (affecting the user permissions page only) by getting rid of jQuery on the user permissions script (core/modules/user/user.permissions.js). The code changes are available here: https://git.drupalcode.org/project/drupal/-/merge_requests/9433/diffs.

I am putting my thread on hold since this issue creating a Drupal filtering library is a much more promising approach.

If I can help adjust the code to a no-jQuery approach, should I commit and submit the changes on the main branch? Or on a separate branch? Let me know what would be your recommendation.

Yes, @mabho that would be great.

The main branch is 3028968-filter-js-library, which has my attempt at JS (I am not a front-end dev, and jQuery is all the JavaScript I know!).

There is vanilla JS on branch 3028968- but that AFAIK is not feature-complete.

Please feel free to make changes to the JS on the 3028968-filter-js-library branch.

1. Added `Search only from start of words` and `Min length of search query` features.
2. Applied library for config-translation

@joachim
Don't get me wrong. You've done a great job here. And I've been inspired by it in many ways. But I think building a frontend library should start with JavaScript. And then adapt it to the backend. And not the other way around. So I will continuing with my approach because it's ready to use right now.

@rkoller
In this task we solve the problem of multiple scripts for the same task, preserving what has already been done. But without adding new features. At least not complex ones.

If there is something that worked before and does not work now, please write here. Otherwise, let's continue working on it in future iterations of this library.

> But I think building a frontend library should start with JavaScript. And then adapt it to the backend. And not the other way around.

I disagree. Complexity should be kept away from users of a system. Here, the users will be developers writing PHP code to apply this filtering functionality to their page. So the API is that part. I don't think there's going to be much need for extending the JS.

I think my approach of a dedicated render element is much better DX than adding attributes. It's easier to document, it's easier to understand, and it also allows developers to control where they place the search filter box on the page.

Here, the users will be developers writing PHP code to apply this filtering functionality to their page.

Users will be any people who will filter tables in the admin panel and possibly in custom modules/themes.
Therefore, in this task, first of all, a simple and predictable JavaScript is needed. And the backend architecture should simply support it.
JavaScript is not a facade here, but a foundation.
As you can see, everything works without a complex API.

In any case, the render element can be added to my MR, and it will be much simpler, as it seems to me. So let's let the community decide how to move forward with this task.

> Users will be any people who will filter tables in the admin panel and possibly in custom modules/themes.

Ok, yes. But my aim at least was to keep the current behaviour, because I felt it had a better chance of this issue getting in that way.

> Therefore, in this task, first of all, a simple and predictable JavaScript is needed. And the backend architecture should simply support it.

I still disagree. The two need to work in tandem.

> In any case, the render element can be added to my MR, and it will be much simpler, as it seems to me.

That would be great. I don't know enough JS to have an opinion about how it should be written! :) I would really like to keep the backend code from my MR though.

Guys, I went over the frontend part of the 'problem', and started off from @innsky's code, which doesn't mean I have any opinion on the ideal approach discussed above. I think the ideal place is somewhere in the middle: keep the backend API allowing module/core developers to provide customization and override the defaults, but provide a set of conventions that would allow users to quickly plug the filter library by using the defaults (table selector, targets, etc.); this way, if no complexity is required, someone could easily and quickly plug the library and have the filters working immediately if no customization if a more complex/custom setup is not needed.

That said, here is the explanation on the changes I applied on the code available in branch 3028968-:

• Use a case-insensitive (this was working already), accent insensitive search (Latin languages use accent very often; I can tell you it is very frustrating when systems don't take accents into account; I am thus 'normalizing' strings before searching
• Instead of searching straight into the DOM elements themselves, I am setting up an array of objects to iterate through. Each array item object carries a lower-case, no-accent version of the string being searched for, as well as a reference to the corresponding DOM element being manipulated.
• I am setting a mechanism to accelerate the queries. Imagine 200 rows to search from. Every time you type a letter, the script searches among the entire 200 rows. Usually, though, users are incrementally typing a word, of multiple search words, and we don't actually need to run a lookup on the entire 200 rows for each keystroke. Example: type string 'con' for 'content' and you are already ruling out most of the potential rows not carrying that string sequence. The next time the script is triggered (let's say, when you completed typing 'conte'), there is no need to start from the 200 records, but we can use a temporary resultset including the ~ 20 search results for 'con'; this tends to deliver good performance improvement on larger tables (not so much on smaller sets, but it still improves the speed).
• The approach with once() is indicating that we can potentially carry more than one input filter per page. If that happens, we don't want all the variables to be duplicated (one instance per input/table). I separated that portion of the code to prevent that type of problem and keep resources usage lower in that case.
• Last but not least, probably the most important change: we don't need to activate the filters if users are not using it; I would say, most of the times users don't use the filter when reaching the users permissions page, or modules list page. I am thus keeping the filter dormant until the filter field is focused. When that happens, then the entire mechanism is activated.

The code is ready on my local environment. @finnsky, let me know if I can push it over your branch as it applies many changes to the original code (the original concept and structure is preserved). Feel free to review, comment and improve from there, and of course, if you don't like it, it is always possible to revert my commits.

Sure please commit anything. Thank you

@finnsky, I am getting a message stating that I am. not allowed to push code to this project. Is this something you can grant to me?

git push origin 3028968-
remote: 
remote: ========================================================================
remote: 
remote: You are not allowed to push code to this project.
remote: 
remote: ========================================================================
remote: 
fatal: Could not read from remote repository.

mabho, if you go all the way up to the summary of this issue do you see this button? (Or search this page for "Get push access")

Looking forward to seeing your code!

I had same problem.
But now seems fine with git and not https

> the next time the script is triggered (let's say, when you completed typing 'conte'), there is no need to start from the 200 records, but we can use a temporary resultset including the ~ 20 search results for 'con';

This and the other improvements in #55 sound fantastic!

Guys, I just pushed the code. This is how I suggest testing this:

Add console.time('timer') and console.timeEnd('timer') to measure the time it takes to execute the event attached to the table instance. I suggest applying the same change to the original code for comparison. It can be applied exactly at this position:

      e.target.addEventListener(
        'input',
        debounce((event) => {
          tables.forEach((tableElement) => {
            console.time('timer')
            tableElement.dispatchEvent(
              new CustomEvent(FILTER_EVENT, {
                detail: {
                  event,
                },
              }),
            );
            console.timeEnd('timer')
          });
        }, 200),
      );

This performance test will measure the time it takes to run each keystroke. It won't take into account the time we are saving by not activating the filter script on each page load.

Let me know your thoughts.

Thank you! I gonna review in next few days.
Please for now fix linter to pass CI js linter and run functional tests.

cd core
yarn
yarn run lint:core-js-passing --fix

Hum, I see the code fails the pipeline, I guess I need to run Javascript code linting against the Drupal recommended standards, I will check how to do that...

Thank you for sharing that information, @finnsky, I will try these commands.

@finnsky, @oachim: I reran the failed functional test from the UI on this page (https://git.drupalcode.org/issue/drupal-3028968/-/pipelines/284678) and all went fine with no failed tests (https://git.drupalcode.org/issue/drupal-3028968/-/jobs/2775475). I wonder if a wait() or sleep() would be recommended at some point in the test script to prevent an intermittent, false-negative result. I also ran the tests locally, and didn't bump into any problems. I assume the script is thus passing the tests.

I am looking forward to your comments/reviews, and let me know if you have any questions, thank you!

@mabho
i've reviewed and tested js. all works fine. Thank you

@joachim
I applied render element to core/modules/system/src/Form/ModulesListForm.php only for now. Same works fine. Please review. I'm not real PHP developer. I'm sure you can do it better ;)
If all fine we need to update other places. They still works as before.

Few notes.

1. I still want to keep ability to use it without render array. For example if i have twig table template and i just want to have quick filter for this. We need to think how to document it well.

2. We need to reapply tests from 3028968-filter-js-library and add Nightwatch tests.

Only few steps left and we are ready to go!!

I don't see how grouping of items is being handled now.

We have cases where:

- items are in a DETAILS element, and if all items are hidden by the filter, then the DETAILS element should also be hidden
- items are in a table, and there are rows which act as subheadings. If all the rows beneath a subheading row are hidden, the subheading needs to be hidden too

- items are in a DETAILS element, and if all items are hidden by the filter, then the DETAILS element should also be hidden

we care about it here https://git.drupalcode.org/project/drupal/-/merge_requests/6309/diffs#d5...

- items are in a table, and there are rows which act as subheadings. If all the rows beneath a subheading row are hidden, the subheading needs to be hidden too

not really sure what it means. all current Drupal filters functional controlled in js.

> not really sure what it means. all current Drupal filters functional controlled in js.

For example, the permissions page. The area to search is a table. There are item rows, and also heading rows.

Items belong to a heading BUT the HTML elements are NOT children!

So when all the items under 'Block' are hidden, the 'Block' table row must be hidden too.

it was added as far as i remember.
better to test for sure.
it was added in January :)

100% it was added into layout builder filter.

There's nothing in the ListFilter class docs or the code about grouping.

Ah yeah, you are right:
https://git.drupalcode.org/project/drupal/-/merge_requests/6309/diffs#a6...

But not sure what to do with it, it will not work with render element.

What do you mean that it won't work with the render element?

I had it working on my branch, albeit with the weird library switching thing I also put in.

yeah. it `may` work` with couple of extra selectors. we need to think about it.

Guys, considering the latest state of the code put forth by @finnsky, what is the filter structure (user permissions, views, modules list, fields etc.) that would require the grouping functionality mentioned above? I tested the latest version of the code on the permissions filter (/admin/people/permissions), and it seems to work alright: only titles associated with the found permission rows are activated/displayed.

There are currently three kinds of structure for filterable lists in core:

- no grouping
- group is an element that contains the children, e.g. details element, or a DIV
- group is an element that precedes the children

If you look at my branch you can see which is which, as I handled this by adding different versions of the library (which I'm sure wasn't a good way of doing it!)

I wasn't aware of the data-filter-label etc stuff, so if that is present on all the instances of preceding grouping, then it looks like that functionality is covered! :)

Btw we can dramatically simplify everything by only using CSS :has() which is in current Drupal browserlist.

@joachim, thank you for clarifying.

From what I have seen, I would assume group is an element that contains the children, e.g. details element, or a DIV is the remaining structure that needs to be tackled/resolved, right? It seems to me the current version is implementing the grouping functionality via group is an element that precedes the children, e.g. a table TR.

Do any of the Drupal core structures using the filtering library rely on group is an element that contains the children, e.g. details element, or a DIV, not covered on the current code?

My question would be: do we really need to support/handle the parent/child approach for any of the core functionality using filters?

> I guess my actual question should be: do we really need to support/handle the parent/child approach for any of the core functionality using filters?

Yes, why shouldn't we? The goal here is a reusable common library that satisfies the use cases.

I've updated the IS with the details of which instances are which type.

Also, I'm not sure why the IS says this:

> Views Add Field popup https://gyazo.com/bd957018d522cf27725c505cfa0ffe7c -- This one seems can be ignored for now. It has extra selectbox for category

We should think about how to support extensions to the library. I don't know how that's correctly done in JS.

I thought to release first version.
With basic features. And then after tests extend it with views with selectbox etc. Otherwise we will never finish it.

> With basic features. And then after tests extend it with views with selectbox etc. Otherwise we will never finish it.

With that I'd be wary of finding after the first iteration that the ways we need to extend it in aren't doable without breaking the API. Obviously we can mark it as @internal to get round that :D

My 5 cents here. You have done a great job, and clearly reached a milestone with the new code: does it deliver everything the previous equivalent, scattered filter versions were doing? The answer, as I see it, is a sounding ‘yes’.

But there is much more. Let’s see:

• It is preventing redundancy (multiple modules implementing the same thing) with different approaches and methods. It thus delivers consistency.
• Users will be loading a single Javascript file for this functionality instead of multiple Javascript files (one per module). Call it a marginal improvement, but we are preventing many Javascript files from being downloaded from the server because users now depend upon a single library for that functionality.
• The new implementation is lean and fast, probably better than all the implementations on individual modules. It saves users' resources, and, considering Drupal's scale, it will help save some trees as well.
• Module developers willing to use a filter can use this approach instead of developing yet another custom implementation.

I would advocate to move this thing forward immediately, and write new individual tickets to improve the code (grouping with children DOM elements; comprehensive API to help module developers extending the code) after it has been merged.

This ticket touches several different files. The more it is delayed, greater are the chances that breaking changes on individual files will require us to analyze the code again and fix conflicts, thus consuming precious time, and delaying things even further.

Quoting Stevie Wonder, this is 'Signed, Sealed, Delivered', what should be the next steps?

Please take a look and confirm or deny

It seems to me that there is not much difference between:

    $form['filters']['text'] = [
      '#type' => 'search',
      '#title' => $this->t('Filter modules'),
      '#title_display' => 'invisible',
      '#size' => 30,
      '#placeholder' => $this->t('Filter by name or description'),
      '#description' => $this->t('Enter a part of the module name or description'),
      '#attributes' => [
        'class' => ['table-filter-text'],
        'data-table' => '[data-drupal-selector="system-modules"]',
        'data-items' => '.package-listing table tbody tr',
        'data-targets' => '.table-filter-text-source, .module-name, .module-description',
        'data-singular' => $this->t('1 module is available in the modified list.'),
        'data-plural' => $this->t('@count modules are available in the modified list.'),
        'data-full' => $this->t('All available modules are listed.'),
        'data-search-start' => 'true',
        'autocomplete' => 'off',
      ],
    ];

AND

    $form['filters']['text'] = [
      '#type' => 'list_filter',
      '#title' => $this->t('Filter modules'),
      '#title_display' => 'invisible',
      '#size' => 30,
      '#placeholder' => $this->t('Filter by name or description'),
      '#description' => $this->t('Enter a part of the module name or description'),
      '#list_container_selector' => '[data-drupal-selector="system-modules"]',
      '#list_item' => '.package-listing table tbody tr',
      '#list_text' => '.table-filter-text-source, .module-name, .module-description',
      '#search_start_of_words' => TRUE,
      '#announce' => [
        'singular' => $this->t('1 module is available in the modified list.'),
        'plural' => $this->t('@count modules are available in the modified list.'),
        'all' => $this->t('All available modules are listed.'),
      ],
      '#attributes' => [
        'autocomplete' => 'off',
      ],
    ];

for a developer.

And this render element seems to be just an extra layer. Which appeared without any real need. What does this give us?

Because that's the design of FormAPI. We abstract away from the specifics of HTML. It's the same reason we have the #options property, or the #label property and so on.

Also, that abstraction is a protection for changes in the HTML and front-end implementation. You might want to change those data-foo keys at some point. Or there might come along a totally new paradigm for setting data on HTML elements for JavaScript to consume.

You might want to change those data-foo keys at some point

It can be managed with setting data attributes with json _encode. It exists in lot of places in core in dialog options. And read them with JSON.parse() on frontend.

          'data-dialog-options' => Json::encode([
            'width' => 555,
            'classes' => [
              "ui-dialog" => "ui-corner-all top-1",
            ],
          ]),

After we release we should never change attributes without deprecation messages.
Technically it is still input type="search". with some extra attributes. Still need reason to have that abstract layer.

> Technically it is still input type="search". with some extra attributes

With extra *functionality*.

Same way we have an abstraction for type=radios, checkbox and so on.

Thank you all for all the hard work on this. Challenging problem and sounds like we need some sort of round table or vote or something perhaps from a slightly wider group. Is #admin-ui slack channel maybe appropriate to have it discussed as part of the next meeting to agree on a direction?

I have been working on ✨ Improve the Search API admin UI for adding/editing fields Active which is nearly there (just coordinating final steps via slack with solr and search api maintainers). As a next step I would love to be able to leverage this as I am sure you have probably all tried search API add fields and know how unweildy that list can become. With search api getting into Drupal Cms (and AI Search leveraging that as a submodule of AI) it'll be a great improvement for both.

So I would love to help test this out within the context of core and test extensibility of it over there (if it's not @internal), but sounds like a bit of a huddle is needed first to know which direction to test.

In any case this is a really great chunk of work that could benefit a lot of areas I'm sure! Thank you!

joaopauloc.dev → made their first commit to this issue’s fork.