This is part of the CSS modernization initiative.
The first issue was regarding the form--field-multiple stylesheet.
The stylesheet at https://git.drupalcode.org/project/drupal/-/blob/10.0.x/core/themes/claro/css/components/form--field-multiple.pcss.css needs to be refactored to make use of modern CSS and Drupal core's PostCSS tooling.
Use CSS Logical Properties where appropriate
Use CSS nesting where appropriate
We need two patches. One for Drupal 9.5.x and one for Drupal 10.0.x
We need a followup issue to refactor this component in Drupal 10.0.x to make use of component-level CSS custom properties.
None. There should be no visual differences.
| Comment | File | Size | Author |
|---|---|---|---|
| #18 | interdiff-16_18.txt | 1.15 KB | gauravvvv |
| #18 | 3294000-18.patch | 1.11 KB | gauravvvv |
| #16 | 3294000-14.patch | 1.25 KB | stanzin |
| #15 | 3294000-nr-bot.txt | 85 bytes | needs-review-queue-bot |
| #14 | 3294000-14.patch | 1.29 KB | stanzin |
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
compare
Comments
Comment #2
sasanikolic commentedNot sure there are any changes needed to this file. @mherchel do you have any suggestion?
Comment #3
mherchelThis one is really simple. I would switch to logical properties and that's it.
Comment #4
aditya4478 commentedCSS logical properties applied for D10.
I think so no changes needed for D9.
Comment #5
sasanikolic commentedLooks good to me.
Comment #6
sasanikolic commentedSwitched to 10.x version because only the D10 patch is posted.
Comment #7
lauriiiThe patch doesn't include the compiled CSS. I'm also wondering if for consistency we should backport this to 9.5.x because PostCSS would provide browser compatibility for this.
Comment #8
sakthivel m commented@lauriii There is no changes in while complied pcss to css, the css code is same but form--field-multiple.pcss.css added PostCSS properties and local variable.
Attached 9.5.x patch
Comment #9
sasanikolic commentedWe don't want to use CSS logical properties like
margin-block-startfor the D9 version, just for D10. That's because in D9 we still support Internet Explorer.Comment #10
ckrinaComment #11
ckrinaComment #12
smustgrave commentedThis issue is being reviewed by the kind folks in Slack, #need-reveiw-queue. We are working to keep the size of Needs Review queue [2700+ issues] to around 200, following Review a patch or merge require as a guide.
Was discussed in slack #frontend channel slightly with @ckrina and @quietone
Moving to postponed as there are still some decisions to be made about how to go about these changes and what's needed in the follow up.
Comment #14
stanzin commentedPatch for V11 :)
Comment #15
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #16
stanzin commentedUpdated patch's EOL conversion to Unix (LF) & UTF-8 Encoding.
Comment #17
aditya4478 commentedLGTM ! Keep going for other issues with same patch format.
Comment #18
gauravvvv commentedUpdated shorthand properties, attached interdiff for same.
Comment #20
Harish1688 commentedHi,
The previous patch #18 failed when tested for Drupal 11. I am uploading the patch (refactor-form-field-multiple-3294000-19.patch) again, for D11. However, I have also tested it for Drupal 10, and it works without any issues.
Comment #21
smustgrave commented#18 still applies cleanly to 11.x and passed the code-commit.sh. #19 was most likely a random failure so the reroll wasn't needed.
restoring status
Comment #23
lauriiiCommitted d7a8c14 and pushed to 11.x. Thanks!
Comment #25
quietone commentedThis is a minor only change, removing tag for the followup.