+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!
Did you mean to comment on a different issue? This is not the place for Drupal CMS feedback.
I didn't quiet get why we're talking about tinymce in this context, could you simplify your point? I can't tell if there is a relation to the core strategy or not.
We're still eager to hear from folks about the new strategy document. Maybe some things are clearer after DrupalCon Atlanta, Dev Days, and maybe there are new things to worry about!
Let us know :)
Looks good to me but outside my scope
reverted for now, thanks for finding the issue!
can't apply as a patch or rebase the MR from the UI, need some manual rebase/merge
There is a merge conflict
Instead of a test I'd be happy with a comment. The suggestion I made in the MR is not good enough but it's a start.
Welcome onboard!
Committed and pushed 119097957c2 to 11.x and 299c60c7f65 to 10.5.x and 18f2872e543 to 11.1.x and 3b341184245 to 10.4.x. Thanks!
The value of htmx for us is in the dom api, not really the implementation or js api. So the magic of other libs will not help us much. We're sticking to lower level stuff in core to make sure we don't artificially limit what you can do. Also less magic translate usually to less code to maintain and happier update paths.
Dom attributes have good DX (from mavo.io that share a similar philosophy) I tried to use that for Drupal a few years back but it's not a good match, it's a CRUD layer, htmx is "just" the http layer with a similar DX and that is very useful for us since the crud part is very much handled in our case
Our ajax api is not intuitive. I have no clue how declare an ajax enabled form element without looking at the docs... HTMX is better than what we have in that regard.
Thanks so much for updating the credits, I was worried about it :)
Spent time reviewing this and I'm going to need a few things:
- Steps to reproduce. I could not replicate the problem on the 11.x branch adding 2 exposed filter on the same page with the same ID, both worked with and without the patch. Not sure how to replicate the problem.
- From what I understood, the test is checking if we can use form 1 and have some results, and check if form 2 gives the same result, I would expect the results to be different, otherwise how can we be sure that form2 worked?
- We should avoid changing
Drupal.views.ajaxView.prototype.attachExposedFormAjaxbecause of bunch of themes overwrite it, let's not make it hard on contrib since it seems doable without changing it
Committed 9bde009 and pushed to 11.x. Thanks!
There are still tabs in the twig file instead of spaces
These issues are going to be obsolete with XB and/or display builder, and in the case of the responsive layout builder it seems that the feature isn't necessesary given the lack of contrib solution for that
#3067700: Bring block class module to core →
#2875935: Add Components Module to Drupal Core →
✨
Add field grouping to core
Needs work
🌱
[META] Add editable responsive layouts to Drupal core
Postponed
#1822950: Add responsive layout builder to core →
#1813910: Add region module to Drupal core (for editable responsive layouts) →
With all the CSS improvements over the years, this is probably unnecessary these days
updating links
we're trying to remove preprocesses so let's find another way, title and icon callback somehow, like how breadcrumbs are manage maybe?
by now hyphenate-limit-chars is now supported on firefox, still not safari.
we could add a rule for german specifically to address the issue without impacting other languages
Thanks for taking additional responsibilities :)
updated permissions
welcome to the team :)
Committed and pushed ab74acc51db to 11.x and 28c4197d572 to 11.1.x and 89375226b40 to 10.5.x and ae2f7f07d26 to 10.4.x. Thanks!
Hello @dan paul your comment looks like an AI prompt, please check the rules for using AI while you contribute to Drupal → .
It should be disclosed that AI is being used and the content reviewed. For example, there is no search feature in the dropbutton. The patch you uploaded is the same, or at least looks identical to the code snippet in the Issue summary. We're not using patches anymore, just Merge requests.
I would prefer not changing the user input since that wouldn't solve the use case of a bad translation. Good news is that the tests shouldn't need to be changed, we only need to change how it's solved :)
A problem in the translation can create issues like 🐛 CKEditor 5 toolbar configuration not show buttons in spanish installation Active .
Also the issue summary and the solution are not the same.
Committed 286ab21 and pushed to 11.x. Thanks!
We're trying to move away from template suggestions so not sure it makes sense to add more for everyone to use, I mean the issue is more than 10 years old no sure it's worth adding something we're trying to move away from.
I'm still seeing 'blacklist' in navigationblock, The change record should present the new option name for the autocomplete as well.
Committed and pushed 0fbf14f118c to 11.x and 02db80612a9 to 10.5.x and 6abbb8516ca to 11.1.x and 709e987ee9f to 10.4.x. Thanks!