🇫🇷France @nod_

Can we change all the similar selectors in the method? otherwise we'd wonder why this one is using css selector and it's using named selectors above.

Among other things, thank you for your help removing the overlay!

Committed faec142 and pushed to 11.x. Thanks!

Thanks @godotislate :)

I'd like someone to test the scenario described in the comment being removed ? Not breaking forms is more important than being valid HTML in this case.

Committed 004cf1e and pushed to 11.x. Thanks!

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

Don't think this needs postponing

Please be careful to not publish the change record if the issue is not fixed.

Yes there are (at least) two strategies :D The needs XB addresses are not the same as what we're addressing with HTMX.

XB is yet to be stable, and I don't know about the stability of the React components but let's assume it's not going to take over the Drupal ecosystem on day 1 :) if you want you can think of HTMX as an intermediary step to simplify the backend, DX, and reduce the footprint of the existing JS everyone in Drupal currently uses. It's more than that but that alone is a good argument for it.

We have already reduced the weight of bigpipe thanks to HTMX. Do we need React to implement Views pagination or ajax forms? I would say no, so moving away from jQuery and unsupported libraries (jQuery form) is something worthwhile to work on. We still have to deal with what people use today and we can make it better for developers and maintainers.

Thanks! can you remove Théodore 'nod_' Biadala (feed) too? or a new issue is better?

we should use the Caddyfile from 📌 Add Caddyfile configuration Active and frankenphp run.

I have some setup steps maybe that could help see how it's running? https://tresbien.tech/blog/drupal-and-php-fun-frankenphp/

Just published a second article :)

nod_ → created an issue.

fair enough, let's go with that

I see that makes sense.

I'm not super happy with the negation, can we have something clearer, it's not obvious that UI = display builders, no_ui could mean the component only output hidden html (script tags) but we do want that to be available in the interface to put on the page? I'm all for consistency but is there a big overlap of folks writting SDCs and FieldTypes/views plugins that needs to be hidden?

I asked some AIs for the naming, got a few interesting suggestions: uiHidden (clearer that the SDC has an output, just we don't want to list it), excludeFromUiBuilders longer but can't really argue about the clarity.

Even got headless as a suggestion since it's for programmatic use, not saying we should use that but it's an interesting take on the situation :)

So we have:

    "no_ui": {
      "type": "boolean",
      "title": "Internal",
      "description": "Use this property to exclude a component from the UI, for internal components."
    },

Why not call it internal directly? We already have a sort of internal prefix for libraries we don't want people to rely on, that would solve the camelcase issue too

fjgarlin → credited nod_ → .

added test

Feedback in MR

Let's do follow-ups, I'd rather get that in regularly than wait 6 months for a big MR.

Committed 93279d3 and pushed to 11.x. Thanks!

The change is reasonable, CR is clear too.

One question on the MR.

Committed and pushed fa5980671e2 to 11.x and 09b2afc5921 to 11.2.x and 036463a2eb7 to 10.6.x and 1ed197bc9df to 10.5.x. Thanks!

good to go, thanks

Committed and pushed 7cf609309d5 to 11.x and ac36d01e3f1 to 11.2.x. Thanks!

Committed 2732a4f and pushed to 11.2.x. Thanks!
We need a MR for 10.6.x and below. Cherry pick isn't clean

Committed 3713df5 and pushed to 11.x. Thanks!

Committed and pushed 72c39ca98ec to 11.x and dce32095c10 to 11.2.x and 0f3a62ab851 to 10.6.x and f0525143a35 to 10.5.x. Thanks!

MR is good to go, The documentation has been read and understood.

Committed 9031a4e and pushed to 11.x. Thanks!

Committed f38f5f4 and pushed to 11.x. Thanks!

updated the CR too, thanks!

Committed and pushed eb0e5f464c5 to 11.x and da5cd09f6f9 to 11.2.x and ec78aecacb1 to 10.6.x and 95d051ff301 to 10.5.x. Thanks!

very much a +1 ! You've been great to work with

We already have a test with multiple body tag, that's why I added the check to the value of the hitTheFloor variable, see test-only CI run: https://git.drupalcode.org/issue/drupal-3533698/-/jobs/5753271

Thanks for keeping things in check!

@fathershawn you didn't misunderstand, I think I didn't explained it clearly enough.

My initial plan was quick and dirty: do the replacement of the ajax commands only so we can get that done quickly and move on. When you added your MR that went with a more HTMX way of doing it, it looked interesting: changing ajax commands for a meta header and raw HTML looked like a great improvement, and you were really involved in making it work so I though it'd be good to try it out. It started to get hard with asset handling and messages. To make it work in that new way the changes and compromises started to become too much for comfort. After chatting with larolwan and catch we figured the less user-disrupting way to go about it was to do the initial quick and dirty implementation. That way if something breaks, we know it comes from a mismatch between what we expect and how htmx does things, because it's the only thing we changed.

Bigpipe is not broken, if we change the data format, how the data is sent or processed we risk breaking it in ways that are not obvious. Check out the history on the big_pipe.js file, there has been many fixes after we moved to the mutation observer handling, it took many years to find the bugs and fix them. I don't want to add more work on this area when there are other things broken we could work on.

Dealing with how messages are handled is going to be easier and less stressful when the change is by itself, not blocking a big feature MR. I second @godotislate assessment the smaller the diff the more likely it is to get committed quickly.

nod_ → created an issue.

+1, see also 🌱 [Meta] Make Drupal the first "design-system native" CMS + Unify & simplify render & theme systems Active

xjm → credited nod_ → .

fix is updating to the new size of assets

There are some unusual things in the test code, needs some clean-up

Committed and pushed e1c818c662f to 11.x and 03a3cf72cc4 to 10.6.x. Thanks!

Committed and pushed 27ad23e01d2 to 11.x and 4a461032dec to 11.2.x and 076888031a9 to 11.1.x and 4f50dadbcb8 to 10.6.x and cb18f9b4fdb to 10.5.x. Thanks!

Much better thanks! the two loops and requestAnimationframes is a good call

Seems like we lost the ability to open closed details elements during search though.

For the class, it could even be a webcomponent now that I think about it. Some previous webcomponent work can be seen here: 📌 Use webcomponents for dropbutton Needs review

I don't feel strongly about the naming, so list_filter is fine with me if that's what people want.

credit

I like how much things we delete, really nice to see!

It's a good start and I have some more comments.

1. Speaking of comments there are not enough useful comments, the places where I need comments to understand what the code does, there are none.

   Declaring a function then an object member with a similar name creates 2 comments and makes the object member comment useless since we can't get the function signature (defaultSearchMethod, and listFilter.searchMethod for example) we don't know the function signature to implement if we want to override this. More on overriding below

2. In the docs inside the JS file I'd like a list of all the data attributes used and the events triggered.
3. The name is not great, we call that list_filter but the code and varibles names are all about table. This is not actually the list it's a seach input with filter an existing list. Something like search_filter_list list can't be the first thing in the name, the render element is not a list.
4. There are a bunch of extension points, but they're not used. For example the only relevant one (in core) would be to override defaultSearchMethod in system.modules.js with the regex, but it's built-in the default function and controlled by an option.

   The point here is that we don't need the extensions points "just in case", this will create BC headaches down the line. This is an internal library, not a package we publish in npm so we can make the api surface as small as possible, so let's do that. The features we want to add to the library and the API surface we want to expose in Drupal core is a problem we already ran into with the autocomplete replacement work, the time it took to figure out the different priorities between the lib and core everyone was burnt-out on it and that killed the effort, let's not replicate that this time.

   So we should only add the extension points we actually need in core. If the regex is built-in the default search method, don't make it extensible. If we want to keep it extensible, let's remove the fromWordStart option.

5. It's really old school JS :) I'd still like to separate the library code and the initialization code. Can you create a subfolder in misc and add that
6. Seems like if there are multiple instances of the filter on the page, things gets shared. Like listFilter.matchedRows, or the two filters can't have different methods overridden, if one overrides the search it would impact the other (another reason to limit extension points). I don't see how we can have different searchingFinished callbacks on the same page. Easier with an event to deal with that.
7. If you don't want to make a class, keep track of the various created instances on the page and all that, save the state in the DOM in some data attributes, for example matchedRows can be a data attribute, makes things simpler.
8. I'm not saying to do it, just asking: couldn't that be a SDC?

few comments in the MR

pdureau → credited nod_ → .

I'll close my branch that replace commands execution code with a htmx-powered code. This issue is way too late for 11.2 so let's try to do it properly for 11.3 :)

AI powered release note

That could be a good idea to only have the "modern" stuff, that would definitely help with BC headaches later on

Yes, but it's a proof of concept, not a roadmap item :)

This is probably going nowhere because our current Ajax frontend implementation is deeply tied to jQuery ajax processing, core and contrib extend it I think we need something new as to not worry about backwards compatibility

Happy to help ! Thanks for being understanding

swirt → credited nod_ → .

nod_ → created an issue.

let's go with removing debounce, the dependency needs to be removed as well.

Committed and pushed 6bdfd060b13 to 11.x and f6ae1909c44 to 11.2.x. Thanks!

I was thinking a contrib module would be a good place to do that, yes.

Is there a security track for Drupal CMS? if there is and there is already a module being used for it might be a good idea to submit a patch to implement those few lines. Or make a module and ask for inclusion in Drupal CMS. Core and CMS have different audiences so it might be more appropriate there

griffynh → credited nod_ → .

griffynh → credited nod_ → .

This could be fixed from contrib, you can change the version from a hook_js_alter:

  #[Hook('js_alter')]
  public function jsAlter(&$javascript, AttachedAssetsInterface $assets, LanguageInterface $language): void {
    foreach ($javascript as &$item) {
        if (!empty($item['version']) && $item['version'] !== -1) {
          $item['version'] = hash('xxh32', $item['version']);
        }
    }

would achieve the same and would keep people off your back :)

Thanks for all the work on the issue, after a discussion with @catch and @longwave we're not going to make this change.

We don't consider the security scanner report as relevant here. Additionally it seems it's only concerned about jQuery version specifically. We have a history of backporting some security fixes to an older jQuery version to prevent core from being exposed (so a site would use a "vulnerable" version, but with our backport it wouldn't be possible to exploit that particular vulnerability). We're usually on top of it regarding jQuery.

Closing and crediting since you've been a great contributor on this issue :)

I spent quite some time working on making just 1 conversion, I agree with #153, this is not the way to go about it. 📌 Slowly, very slowly start OOPifying the render system Needs review is a much nicer solution.

porting credit

Was able to cherry pick after backporting the other issue

backported the issue, and rebased the MR

backported to 11.2.x for 📌 Update prettier/PostCSS/stylelint for 11.2 Active

that looks reasonable

Committed and pushed 34f55547a4e to 10.6.x and 7d1886dab79 to 10.5.x. Thanks!

This changes the query parameter from v= to hv=, it shouldn't impact much things but I'd like an other review to make sure we don't have too much things depending on the query string name.

Committed and pushed 7cf46468f46 to 11.x and 103f4c829e5 to 11.2.x. Thanks!

yep, thanks!

Committed and pushed 5aa1596bdc3 to 11.x and 19df4f1774c to 11.2.x. Thanks!

somehow doesn't cherry pick to 11.2.x

small nit about the placement of a comment

I think there is a timing issue sometimes. contextual.js declares Drupal.contextual.ContextualModelView as an empty object and it get ovewritten in contextualModelView.js with the class.

Just trying to put everything in one file and see if that'll fix the problem.

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

It's green:)

I profiled the change and this is a terrible, terrible idea. it's 10x slower and uses 10x the memory.

Adding just in case

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

nod_ → created an issue.