🇫🇷France @nod_

The issue comes from a slack discussion which was in essence. Let's put UI suite in starshot to let users try the "site build able components" parts of XP builder now rather than later. So a bit redundant to put that here imo, but it can't hurt

+1 from me

Very much +1

Since we use jQuery UI we have to use some js for positioning. If we had native dialogs it would be easier to go with css-onlu for positioning.

haven't tested it yet but code looks good, I'm +1 on using floating ui for that

pameeela → credited nod_ → .

pameeela → credited nod_ → .

Thanks! That's an interesting project. Haven't had time to dig into the code but happy to see alternative takes being explored.

I'm personally for htmx because the philosophy is aligned with Drupal and what we try to do. And I like that a lot of things live in the markup instead of hidden away In a js object somewhere.

Agreed with #5 about scope. For the poc a new #htmx key sounds fine but we'll need to make existing code work as-is as much as possible down the line

can we add a comment as to why we need to have the height changed in the CSS? I'm sure i'll wonder why it's there in a few months.

Committed and pushed 7ac4979e2e to 11.x and ccc368e015 to 11.0.x and 26aafb10e3 to 10.4.x and cc27b4df99 to 10.3.x. Thanks!

Committed and pushed 799802965e to 11.x and 53c88323af to 11.0.x and 5d9fa4ef0d to 10.4.x and 4db4f4b4b4 to 10.3.x. Thanks!

Haven't tested it yet but I like this much better, thanks

umm maybe that needs to be a follow-up issue

I'm not sure this is the right fix. I can still get the menu icon offset with the patch on the desktop resolution with the menu collapsed. it does fix the mobile use case but I think we need to fix it once for all the use cases.

I think the root of the issue is this rule:

body.is-fixed .container {
  width: calc(100% - var(--drupal-displace-offset-left, 0px) - var(--drupal-displace-offset-right, 0px));
}

This doesn't seem to work like expected. this would make sense for absolute/fixed positioned elements, but .containers are not, from what I can see they're managed with flexbox, so we don't need to account for anything special.

There is another issue with displace, there is a 9px offset on top, when there should be none.

Committed and pushed c0beeeea5f to 11.x and 6974f8b7f0 to 11.0.x and 04fad10830 to 10.4.x and 2237b2eed8 to 10.3.x. Thanks!

Committed and pushed 924d567aac to 11.x and 1e0296d353 to 11.0.x and 4c8f3371e4 to 10.4.x and 57e4625a18 to 10.3.x. Thanks!

This needs a default value for the css variable. If displace is not used on the page (no toolbar or navigation module) the skip link takes the whole width instead of the body width

Fixed css lint error on commit

Committed and pushed 5e0d1efa0a to 11.x and 0b36d01ad6 to 11.0.x and c1f7726488 to 10.4.x and 12b546ca74 to 10.3.x. Thanks!

Committed and pushed f5cc65456a to 11.x and cc355625ad to 11.0.x and 05b6144bd0 to 10.4.x and 270f95ac48 to 10.3.x. Thanks!

works for me, thanks.

The existing tabbingmanager feature has tests, this one should too

Thanks for the patch, I'd rather keep and try to solve the issues raised in the MR. Setting back status from #14.

nod_ → created an issue.

This fixes it in claro, need to fix umami, it does the same center positioning think as before. Olivero is fine (design is different but it's top-left aligned already)

RTBC looks fine, Can I get the changes in a MR please? Thanks!

Committed 91e3259 and pushed to 11.x. Thanks!

Committed and pushed e8298e81ba to 11.x and 22b06e0042 to 11.0.x and 8e344072b3 to 10.4.x and 271a556170 to 10.3.x. Thanks!

we can see in the screenshot from #23 that the padding is different in the dialog. Is there no way to avoid adding specificity to the .button--danger selector? BEM is supposed to prevent having to do that.

didn't review everything, what i saw looked much better, thanks :)

So something like having a lock set on cache clear, and only serving the assets without writing to the folder when the lock is present?

As one of the people tagged in #35 my response in #17 was pretty short, longer version: I'm all for it. I don't think the other want to keep the existing code either. Next step would be to open an issue with a PoC, (this issue is more for meta-talk, i'd rather have the discussion of implementation in it's own non-plan issue). Iterating in contrib is a good idea too, FYI I personally won't have time to be involved outside of core for the next few months.

For the PoC what I have in mind:

• One or two example of a replacement in core (at least)
• A list of the various ajax commands we have in core and their feasibility (with a very rough complexity estimate easy/hard) with htmx
• Test runs don't need to be green, although it' be nice if a couple of them were passing.
• can we use this to handle ajax form submit? if yes, how complex does it look? would be great to drop our jquery formsubmit fork
• A big thing here is to improve DX, so a couple of examples of how it could make it easier to introduce some ajax stuff in contrib/projects would be good.
• rough dependency evaluation https://www.drupal.org/about/core/policies/core-dependency-policies/depe... → (being a great meme poster is not enough :p)
• BC strategy (support existing calls, or a new codepath altogether, etc.) I'm not too keen on two separate mods. I feel it'll make adoption much, much slower than spending time on the BC layer even if it takes a long time. (we can always put restrictions on what the BC layer can do) if we have 2 code paths, we'd need a contrib module to bridge the two and let contrib maintainers test with htmx easily without changing their code

Feel free to open an issue already, you don't need to have all the above to get started and send code, focus on the fun parts for now :). It's just a list of the things we'll need at minimum on the way to a core commit, it'll probably be split up in several issues once it's mostly ready to make review/commit easier. I checked the code for the contrib module, some stuff could be simplified on the js side (you don't need to attach the eventlistener to the body in a behavior for example) all in all, looks promising.

I'm expecting a lot of discussion in that issue so please have in mind it'll be a marathon, not a sprint. Ajax support in core is ancient and changing it might be challenging, but it's worth it and I'll be here to help along the way.

Committed 91b4cf2 and pushed to 11.x. Thanks!

can I get a rebase/merge of 11.x here please? I know the merge request works but it doesn't apply cleanly as a patch.

Committed bd81352 and pushed to 11.x. Thanks!

Same here, the JS is ok with me for a Beta, for stable i want to use proper events like we did for dialog but that can wait (since we'll be marking all the JS apis internal for now), there is generally too much nesting, some things can be refactored but nothing impacting the user experience, it's mostly about DX and maintenance. Better get this feature in users' hands first.

Agreed on having everything internal. Can we make the css-only libraries internal too? just in case. We can always make not internal for stable realease.

I would define floating ui as internal too, it's in the assets folder but it's not defined in core libraries hence shouldn't be relied upon just yet. I wouldn't want contrib to start depending on navigation/floating-ui. If we don't want to make that internal, that needs to go in core.libraries.yml

Other than that, +1

Thanks! Published the change notice, does it need release note?

I agree with the diagnosis and the end goal, and I think shoving components in template is a necessary step.

We do not have a way to site build the components in core yet. We shouldn't wait to have it to make the components so having template call components is a way to let the community start the components and use it in core. When we have the capability to site build all this yes we need to make use of that capability and stop hardcoding the components in template.

that's at least major given the impact

Committed 20d5c8a and pushed to 11.x. Thanks!

Committed dc7a429 and pushed to 11.x. Thanks!

awesome, there was a conflict in libraries.yml on the backport, opened a MR for 10.3.x

I was being too cautious limiting to 11.x (was still provisional at the time, might be part of the reason too).

So yeah, 11.x and 10.3.x otherwise it'll be hard to remake a patch

Committed 74f01af and pushed to 11.x. Thanks!

got the same results building css locally after a build:css.

-webkit-appearance vendor prefix is fine to remove, it's for old unsupported browsers

I'm committing a bunch of issues with css changes to make the RTBC queue go down, so we'll have to reroll that just before commit.

Committed 9665396 and pushed to 11.x. Thanks!

I was worried having drupalSetting added to the preview could break views ui somehow. Haven't found an explicit way it could yet.

Not really my area of the code so leaving the commit to someone else. RTBC +1

Committed 299f4c8 and pushed to 11.x. Thanks!

I think that solution is pretty clever, could be good to have in a blog post somewhere :)

Committed 3f7d286 and pushed to 11.x. Thanks!

raising to major since it interferes with the normal use of the site.

We should be looking at potentially splitting the font up to reduce size see https://github.com/zachleat/glyphhanger

Committed 296e471 and pushed to 11.x. Thanks!

about #60 given that this is an automated MR, I'd be reluctant to introduce manual changes on top of it. I did it for the once() replace patch and don't recommend it because of the difficulty to reroll. IMO The comments were placed awkwardly to begin with, so it make sense that an automated fix would place them not where we want.

Given that this is an automated patch, I'm fine with the comments in this issue but a follow-up needs to be created to move them either above or before the opening bracket.

Since we'll use prettier for css, we might want to enable css file rewrites in the prettier command of our package.json. Ok with a follow-up for that since we'll need to exclude a bunch of files/directories from this auto-formatting and it'd make all this longer.

all good with me. I'm ok with the variable being in the follow-up, the keyframe stuff was going to be my next question :p let's do that in the other issue.

@Pravesh_Poonia I'm not sure what you mean by "But checked MR, changes looking fixed now" . Your text looks automated/generated, could you elaborate as to what you were trying to do? Thanks

Minor so no backport

Committed fec3962 and pushed to 11.x. Thanks!

I was involved with the code so I wanted to have another person RTBC so that I could commit but it doesn't seem to help get that in. It's a 11.x commit only.

Awesome, thanks a lot for the clear issue summary and the MR!

nod_ → created an issue.

Committed 41943e8 and pushed to 11.x. Thanks!

Maybe for the js-hide class we could use the media query to make sure we hide things asap. Would be good to try it out see if that helps some UI

Committed 8ac73be and pushed to 11.x. Thanks!

Retitle for commit, opened 📌 Refactor (if feasible) uses of the jQuery parents function to use vanillaJS Active to address the other uses of parents and change the eslint config

nod_ → created an issue.

The issue is with Js being half-available. if some of the JS execute but not the rest the feature could stay broken, see #3441124-12: Views UI action buttons create janky layout shift on page load → for a possible fix

ohh clever, i like that. Don't hesitate to make a class or something out of it. All the MR doing the same thing will need it I think

Thanks for the work!

:visible will be tricky to get right so I'd rather leave that in for now, we can always fix it in a follow-up. In some places there is a bit too much of code rewrite, if we just need to update the CSS selector it's better to just do that and not try to replace addClass and that kind of things.

Putting back at NW to make sure that in case JS crashes or can't downaload completely this still works.

credit doesn't happen automatically for commit if you set it fixed before the commits show up on the issue :)

I think the needs review bot should work better.

I agree, not much time for it though

looks good to me, bot is just getting in the way. it's not too smart to change the issue major version fast

Committed f45c7bd and pushed to 11.x. Thanks!

awesome :)

will most likely need a 10.3.x MR since it won't cherry pick :D

How does it recover if the js crashes or doesn't finish downloading?

seems reasonable to me, and when we get over it we increase by 500b or something like that?

reran the failing test, green now.

green

I'll take care of it, thanks :)

failing on 10.3.x because of asset size test

Committed 449f6fb and pushed to 11.x. Thanks!

With the patch in the current version (using claro):

1. item list style missing /admin/reports/fields, the new library is never attached anywhere
2. tablesort indicator icon missing on claro /admin/content?title=&type=All&status=All&langcode=All&order=status&sort=asc this might need to be a follow-up, looks tricky.
3. no style on status page /admin/reports/status
4. There are image paths inside the CSS files that needs to be updated since the path of the css file changed.
5. Textarea resize styles not applied /admin/structure/types
6. Haven't found a test case for fieldgroup but that won't work either

The 2kb makes sense, there is a bunch of file that should be loaded that are not :D

rebased

leftover comment, possibly when the method became an anonymous function

thanks, that works, we can always improve later if needed, not breaking stuff is more important :)

Committed abf79d7 and pushed to 11.x. Thanks!