Committed and pushed 09656a796c7 to 11.x and e0f3aad5bab to 11.2.x and 3c7cc933c02 to 10.5.x and 4f88d508615 to 10.6.x. Thanks!
Tried it out and that is the appropriate fix. Even if it's not in a noscript element it just makes sense to check if the element exists before trying to run things off it.
About #34 and #39, adding a @ in front of the user name would link to the user in the gitlab UI: https://git.drupalcode.org/project/drupal/-/commit/6ba27a8298b3bdf05bab4...
[#122312] feat: something something
By: @nod_
Would transform to a link to my user when reading the commit message, just like in MR comments. I tested the standard format and pictures are broken for some reasons as you can see here, and what gitlab links to is a mailto link, not even the user profile: https://git.drupalcode.org/issue/drupal-3548100/-/commit/4ef708e0473351b...
So either we keep it as bare username or prefix them with @. The actual standard format doesn't look very helpful for us.
started to go another way in 📌 Ajaxify the user interface translation forms Active
Instead of handling that in the backend, the query string is added from the front and we have an extra drupal-specific attribute that controls it
I think I found the sweet spot for using/cleaning the extra parameters in 📌 Ajaxify the user interface translation forms Active . If wrapper format is not a problem we still have to manage ajax_page_state.
Also if we avoid dedicated endpoints for views, we should be able to simplify the backend too.
I agree that using _format is a bit of a stretch so I don't mind a dedicated key.
I'm leaning towards using boost with a drupal-specific attribute to control the wrapper format, ajax page state and push url.
Seems like it! the boost attribute does more than I need though,
Checked it out more closely, we kind of have nested attachBehaviors call, that isn't idea.
Instead of that we can just try to call the function "safely". So doing
$(document).on('drupalContextualLinkAdded', (event, data) => {
Drupal.ajax?.bindAjaxLinks(data.$el[0]);
});
and keeping the backend code that conditionaly load core/drupal.ajax should be enough
Oh nice, and the perf test looks great with 27k of JS removed :D
Tests are green, since we never used fetch directly in core before the test framework didn't know how to wait on it.
We should be able to find a way to make the ajax contextual link not a dependency. Does anyone has a example of this being of use? was it for quickedit back in the day maybe?
This MR builds on 📌 Improve url management of the Htmx object Needs work , 📌 [PP-1] Refactor ConfigSingleExportForm to use HTMX Postponed , 📌 Ajaxify the user interface translation forms Active , which is why the diff is important.
To test, toggle the "use ajax" setting on the view you want (I only tested the /admin/content table and exposed filter at this point).
Instead of a custom _htmx_route, maybe we can use _format: drupal_htmx or _format:htmx in the route definition.
See how lupus_renderer handles that, creating a new _format:custom_elements for api responses.
The new tests are still passing and it makes sense based on upstream code,
updated the version in the change record, it'll be 11.3
Committed c8dae79 and pushed to 11.x. Thanks!
I don't think that warrant a change record, it will just work™ now
Committed 103108a and pushed to 11.x. Thanks!
should be good to go, maybe comments needs some work? not good at that.
Played with the patch a bit, the attribute stuff I'm doing with htmx doesn't break. And things seems in order in the admin UI. I asked @grimreaper to test it with ui_patterns/display_builder since they might do more complex things with twig than core. Maybe we need to test twig_tweak too?
got the pagination working but it's not limited to this form, I don't know what else it might break
Got it sort of working but there are problems:
- Pager links are very broken, they get all the form stuff added to the query string and since we exclude the libraries trying to navigate to them is just not working.
- The session values never gets updated so default filtering stays at what it was. When the page is refreshed we don't get the last filtered thing.
We should be able to deal with it here, need to try a few things where we might have to refactor the backend a bit
Now that we have htmx in core it could be a bit easier to manage that. We don't use htmx for exposed forms but that should be something to look into
Sorry late to the follow-up. Since we have different rules between 10.x and 11.x I'd rather avoid the inconsistency and not backport this.
Thanks for the dedicated MR though!
not backporting to 10.x
Committed and pushed 3a9f2bab7e5 to 11.x and 17f7df3e200 to 11.2.x. Thanks!
Committed and pushed 2c8c0a2efb2 to 11.x and 2042dcf3a4f to 11.2.x. Thanks!
Committed and pushed 61e09f28155 to 11.x and 8d86de67efe to 11.2.x. Thanks!
I have a problem with this line, There is not a finite amount of values for $definition['provider'] passing that through t() is not a good idea.
$definition['category'] = $this->t($this->getProviderName($definition['provider']));
The rest is fine.
@fathershwan: it sure will, that's the point and that's why we filter out the Drupal specific pieces out of the get parameters. We need that for TranslateFilterForm at least.
If we have another method in the Url object or somewhere else that does this I'd be happy to remove this chunk of code from the Htmx object, but I don't have a good place for it somewhere else
that's my mistake, #3546670: Missing module version in 2.x branch → .
Thanks for the lightning fast reactivity!
That's my bad, I was on a dev version and when upgraded to the beta the release was not picked up, it used the git version of the source files.
Sorry about the noise!
I think the problem is that there is no module at the root of the repo and the packaging script maybe doesn't recognize that so it doesn't add the version automatically?
I do not know how to fix is, I asked on slack. Since the layout of the files is a bit different maybe it needs to be ignored?
This issue removed the version for the module, now there is a warning for country_path because there is a version constrain on domain and domain doesn't have a version a Drupal website understands anymore.
I attached the push url to the form itself, since it's not about the field itself it's more about the whole form. And refactored a bit to make only one call to the pushUrlHeader.
I've spent days on this form and an other version of this form is used in the Htmx tests so I'm confident we got everything working. it's also an improvment compared to before because we can use the back button of the browser now :)
I'm not happy with that static method being the only way to add the wrapper format.
We can add the static method but I want to add the htmx wrapper format by default to the Urls the Htmx class handles, and provide a way to opt-out when necessary. The wrapper format is an implementation detail of Drupal, people using Htmx in Drupal shouldn't have to know about this.
Making a test harder to write, and in exchange we won't have to explain anything about wrapper formats to users is a good compromise to me.
Because of how these forms work we need 📌 Improve url management of the Htmx object Needs work before
created a follow-up to make it easier to use this wrapper format: 📌 Improve url management of the Htmx object Needs work
depends on 📌 Return htmx responses as SimplePageVariant Active
small change, we can declare the settings in drupalSettings in the library definition, that will make sure we have that available on the frontend side of things:
drupalSettings:
# These placeholder values will be set by xxx::preRenderPlaceholder().
contextual:
theme: null
excellent :)
I'm hopeful we won't much more htmx specific code in the backend side of things.
What I have in mind with regard to htmx: make the form work without JS and make htmx the ajaxify-ier of that form. We're currently looking at some ways to keep the callback mechanism of the ajax framework without the whole separate processing on the PHP side. I'm almost sure we could have that type of callbacks for non ajax forms, need to look into it more, in any case if it makes form processing more complex we're not going the right way. Fun times ahead :)
Can someone confirm which MR is the one RTBC (12460?) and that the comments from MR 7008 are addressed in the latest MR?
nod_ → changed the visibility of the branch feature/3049332-9.5.x to hidden.
nod_ → changed the visibility of the branch 11.x to hidden.
Committed and pushed f01ae18dfdb to 11.x and 08bc5ec4746 to 11.2.x. Thanks!
I'm not sure the fix is appropriate.
IDs are not a backend concern, so it seems strange to handle that in the backend when the frontend can deal with it, we try not to rely on ids for styling and JS so the id could be random and it'd still work.
The MR https://git.drupalcode.org/project/drupal/-/merge_requests/6965 looks promising need to use |clean_unique_id to make sure things don't break when you're dealing with ajax stuff.
Seems about right given #3374162-7: Make it work with AJAX enabled →
I think i got them all now
When destroying the list filter the filtering needs to be turned off too, currently the list stay filtered and we can't access the whole thing + comment in MR
@fathershawn was right about build id replacement :)
I removed the id match because we can actually be very specific about what we replace. This will avoid instances of people adding an element on the page with the same id as the form_build_id input just to mess with things.
Checked that the feedback has been addressed
the destroy step is not quite there, comments in MR
Back to RTBC, we have tests
smustgrave → credited nod_ → .
That's Drupal Canvas
nod_ → changed the visibility of the branch 3535173-htmx-aware-forms-tests-only to hidden.
nod_ → changed the visibility of the branch 3535173-support-dynamic-forms-only-with-test-fix to hidden.
nod_ → changed the visibility of the branch 3535173-support-dynamic-forms-only-with-test to active.
nod_ → changed the visibility of the branch 3535173-support-dynamic-forms-only-with-test to hidden.
nod_ → changed the visibility of the branch 3535173-support-dynamic-forms-only to hidden.
We should move the changes to the config form in a new issue like we have for 📌 Ajaxify the user interface translation forms Active so that we can use the new Htmx object so that we can start using the helper.
Just rebased the MR so that it'll work with the last version
Works for me!
Discussed on slack and I'm +1 for the simplification
Just a sidenote about the attribute class
/**
* {@inheritdoc}
*/
public function offsetSet($name, $value): void {
// If the value is not an AttributeValueBase object, then create one.
if (!($value instanceof AttributeValueBase)) {
$value = $this->createAttributeValue($name, $value);
}
$this->storage[$name] = $value;
}
A little problem with this is that $name could be different than what's $value has been created with. Not sure why it'd happen but it's very easy to run into.
In any case I'm happy with the MR so RTBC +1