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.
I profiled the change and this is a terrible, terrible idea. it's 10x slower and uses 10x the memory.
Adding just in case
Made things a bit clearer
Small nitpick for the selector, fix look appropriate
There are not that many failures in the daily build. This is breaking something else somewhere.
I don't understand why we need the single quote. If you add a JSON::encode(), to the process assets MR it just works.
$build['replace'] = [
'#type' => 'html_tag',
'#tag' => 'button',
'#attributes' => [
'type' => 'button',
'name' => 'replace',
'data-hx-get' => $url->toString(),
'data-hx-select' => 'div.ajax-content',
'data-hx-target' => '[data-drupal-htmx-target]',
// add this part
'data-hx-vals' => Json::encode(['test' => 'ok']),
],
'#value' => 'Click this',
'#attached' => [
'library' => [
'core/drupal.htmx',
],
],
];
It doesn't matter that the source html doesn't look pretty. Once it's in the JS it's the correct value.
I agree with tom konda :)
Committed 89b4900 and pushed to 11.x. Thanks!
Committed and pushed a6ad55b2765 to 11.x and 110c8468c25 to 11.2.x. Thanks!
This is a feature, not a bug.
Check out the 3 lines of comments just before the code change:
// Preview the machine name in realtime when the human-readable name
// changes, but only if there is no machine name yet; i.e., only upon
// initial creation, not when editing.
You want to be able to change the label without changing the machine name. Happens all the time that some vocabulary is renamed to be clearer to end-users but you want to keep the machine name to not mess with config export and custom code.
Thanks, there are comments about IE6, any plan to get rid of that too? :)
Committed 24dcc8d and pushed to 11.x. Thanks!
Don't mind me, just fixing the version.
I like the idea. Hopefully we don't have too many forms where content and configs is mixed up.
Might need UX folks for how this should look like and behave
Voting for:
- Marine Gandy (Mupsi)
- Joris Vercammen (borisson_)
umm but then we get duplication, I think it's fine, details is not really a heavily customized template
We should be able to do that from twig only, no php changes needed to form.inc
FrankenPhp is going to the PHP GitHub org:
https://externals.io/message/127347 with some of the doc moving to PHP.net
This one can still go in 11.2, if we don't have this one, the htmx lib is useless which is unfortunate
added tests for CSS files and JS behaviors
Fixed a bug where ajaxpagestate was not added to the page when using only htmx, when core/drupal.htmx
library is detected on the page we add all the ajax-related infos, otherwise we end up with JS/CSS files being loaded twice.
Should be good to go but since I added a bit of logic for drupalSettings stuff, putting as NR
I think that's what finnsky had in mind too
@supriyagupta: I don't understand why you created a fork and the content of your comment, could you clarify please?
@alex.skrypnyk: When I opened the ticket I did not understand you meant exemples yet, so the retitle is inline with what I had in mind, which are the use cases explained by Pierre above.
We need to add ajaxpagestate on all pages that have drupal/htmx library otherwise things will break in various use cases. working on it and adding tests for adding css file, js module, attach and detach behaviors
There is suggestion in the MR
small feedback, unnecessary nesting. You can RTBC once it's fixed
Committed and pushed 2f17d640524 to 11.x and dd94411ba01 to 11.1.x and 21bc1da3e70 to 10.5.x. Thanks!
Tried to simplify some things.
For the credential forms we don't need to change too much logic, we need to reverse some conditions and it works as expected.
+1 for the two landmarks
We need a change record with code examples to show when the condition in #38 is likely to occur and code sample to show how to fix it.
updated the code, can you check it solves the problem?
that's a good idea, follow-up though
The lock file is correct, latest version is 2.0.4: https://www.npmjs.com/package/htmx.org?activeTab=versions
The debug extension is in the package because of how they used to do things: https://github.com/bigskysoftware/htmx/blob/master/dist/ext/README.md
let's remove the debug extension, it's not used yet and there is a built-in way of doing the same thing (htmx.logAll()
)
as for the folder that's my bad, I removed the "folder" key and we needed it
All good for me, critera makes sense.
With what's going on with chrome/google in the US and the AI turn everything is taking we might not need this for a while but it's good to have it laid out.
yes sorry missed it in the issue summary
The problem here is using the browser local and not the configured user interface local to format the date.
Have one editor use a browser configured in french, another editor with a US browser and they will never agree on what the created date is. I would be inclined to won't fix this. The current format is not pretty but it is never ambiguous.
had a comment on the MR waiting for several weeks, sorry about that
Committed and pushed 19f88bd3c67 to 11.x and d0c9e27897b to 11.1.x and 5d36123b2a5 to 10.5.x and 3ff0b26d975 to 10.4.x. Thanks!
Or could this be a follow-up? since the new proposal is to add "props" to what we already have in this MR, feels like something additional to the current approach and not fundamentally incompatible? Having the variant "leak" into the twig template might not be ideal but it might still happens with the new approach for some Drupal reason.
Opened ✨ Add component variants to SDC, storybook style Active to discuss the new proposal. It's a terrible issue title and issue summary but it exists :)
As far as I'm concerned this issue is still RTBC, the approach is to declare variants the same way slots are declared, it's easy for people writing the sdc yaml to remember, title/description for metadata, variant name available inside the twig template. The MR implements that correctly.
Now we're having some of the assumptions being questioned. Variants should be a predefined set of props, and variant name should not be accessible from the twig. There are many questions I have with the new proposal and to not derail this issue further I opened ✨ Add component variants to SDC, storybook style Active to discuss.
If we think that the new approach is better we'll close this one and credit all the folks that participated. If the new approach doesn't work out, we'll go ahead with this one as-is.
How would it work when you define a value for a prop in a variant, then call the template with a different value for that prop? does the variant setting win over the assigned value? Do we trigger a warning when a prop defined in a variant is assigned when calling a template directly?
Much clearer, thanks
I don't understand what you're proposing.
The implementation here does not prevent anyone from implementing variants as you describe in contrib. It's entirely optional so you can not use it and implement something else, or am I missing something?
I can agree that variants could be a set of things defined outside of the sdc, but we don't have another intermediate level that can be used to describe what the variants are.
Just to clarify the code smell you pointed at is not in the MR.
Simplified as much as I could, seems to work as expected.
The very minimum is to move the .trigger(resize) code out of the tableresponsive constructor. Add that after the once() in the attach behavior and it should mostly fix the problems of runaway initializations. We can also add a debounce to prevent too many calls to that function. The resize event is explicitly mentioned on the debounce script as a valid use case :)
Thanks for the MR!
Glad to see this moving fast. Had a bunch of comment on the MR. Generally looks like things will work out :)
a proper fix is in the works for core, could you test 🐛 Form field #states not woring with entity reference fields Active . It should fix this but introduces another change in that #states about a missing element are now applied, they used to be ignored.
a proper fix is in the works for core, could you test 🐛 Form field #states not woring with entity reference fields Active . It should fix this but introduces another change in that #states about a missing element are now applied, they used to be ignored.
a proper fix is in the works for core, could you test 🐛 Form field #states not woring with entity reference fields Active . It should fix this but introduces another change in that #states about a missing element are now applied, they used to be ignored.
MR is green, need the comments added back for the various function we copy/pasted.
Yes so about the code, it works, I don't see any memory leak (with a moderate usage) so that's good. The touchy subject is from #38:
The current fix feels more correct to me, in that for the state to be applied, the trigger element must first present in the DOM and then the trigger condition must be TRUE. Otherwise, the inverse state is applied. However, this is a change in states behaviour, so it would be good to get maintainer feedback.
I'm a bit uncomfortable with that change, it does feel like a big change and people will wonder what's going on. I'd like to have some webform or other heavy users of #states involved here because I don't want to break all #states forms that are not correctly written. Like if you have a typo in the states config, it used to work and now it'll be hidden because of the typo.
I agree it's better this way, but given how old this thing is, the less than ideal way might be the way to go.
could reproduce the problem with ajax using contextual_test module.
It seems it's the event delegation that used to be in backbone that was making things work. Somewhere it was switched to direct event binding and because there is so much re-rendering the event listeners were trashed and it caused issues apparently.
Tried to go back a few commits and fixed a problem with hovering out of a region and contextual links not closing, not sure it's the same issue as #51 but it was broken before and now it's not. Can you let me know how to reproduce the failure with dialog?
opened a new MR to not erase the work in case it's a different bug.
Also all the comments have been removed, would be good to keep them around, they help with understanding what's going on
Can you point to documentation about "Elements" ? Curious about the requierments
We didn't have a change notice to say a library was added (like sortable and such), only the issue when it's actually used has a change notice.
For now we don't want people to use it as-is, we need to integrate it to make sure things like security and performance are taken into account.
nod_ → created an issue.
Since there are upgrade path concerns, tagging for RM review
+1 for this, validated that in #3404409-99: [Plan] Gradually replace Drupal's AJAX system with HTMX →
Crediting htmx maintainer for resolving the security policy issue
doesn't cherry pick to 10.5.x, need a MR for that one
Code is good to go for me, we can RTBC once the security issue is managed. To be more accurate the status should be postponed since we're waiting on external third party for a reply :)
I'm happy with that
few comments on the MR
Variants are not the only concepts, we also have styles and themes/modes. Next step is ✨ Add a style utility API Active and XB will need something like https://www.drupal.org/project/ui_skins → sooner than expected too.
So you're right that variant are not the only dimension, it's just that the other dimensions are not at the component level, they're a higher level concern.
@joseph.olstad: I asked for a simplification and you posted an even longer text, this is not really helping me understand your point. I'll assume the simplification are the last 4 bullet point and I won't engage with the rest of the comment.
So you think the current strategy is good and it's "just" missing the last 4 points you outlined or should the strategy only be the last 4 points you mentioned? Help me make sense of what you mean. If it takes even longer to explain, take a step back and simplify please, otherwise I'm afraid I'll have to ignore it. I'm looking for something like "I agree with the strategy but it's lacking X,Y,Z" or "I do not agree with the strategy because X,Y,Z".
The PDF document represents days (possibly weeks) of meetings, I'd appreciate if you had the curtesy of applying this same spirit of concision to your feedback.
@jonathanshaw: Thanks for the feedback. It's true that AI was not really a topic we discussed in depth in the various meetings. As we can see with the vibrant activity around AI, Core does not seem to impede progress on the topic?
I agree with you that the points raised are not really about the strategy. It seems to me like you don't find the strategy bad, but that there is not enough mentions of AI in there ?
Points 1 and 2 of the core section in your comment would definitely help humans too, for 1. we do have a plan it will be made more explicit later this year as there are other things to adress before that. for 3 core could do that, but it can be started way, way (way) faster in contrib and as far as I understand speed is important in that industry :) Core is neither enabling nor preventing progress. If it is, point me to issues and I'll have a look. For 4, personally I'm keeping my head in the sand for a while longer, I'll let others tackle that for now :)
On a more semantic note, I find that talking about war on the topic of CMS priorities a bit misplaced given what's happening in the world. I understand the feeling but yeah, I don't feel great about the term used.
@all: To frame the discussion as I said in #7, for now the focus is on the overall strategy. First do you agree with the goal? it might not look like it but it's a pretty big change to become a CMS builder (and not "just" a site builder). Did we miss anything in the "where will drupal core play" section? any worries about the realignment of Drupal Core with regard to Drupal CMS, with the CMS market, or even with the broader open-source and commercial world. Does the "how will we win" section make use of all the strengths of Drupal in general, are we missing something to achieve the goal? Thanks!