Refactor Claro's form--text stylesheet [#3294006]

Problem/Motivation

This is part of the CSS modernization initiative.

The first issue was regarding the form--text stylesheet.

Steps to reproduce

The stylesheet at https://git.drupalcode.org/project/drupal/-/blob/10.0.x/core/themes/claro/css/components/form--text.pcss.css needs to be refactored to make use of modern CSS and Drupal core's PostCSS tooling.

Proposed resolution

Use CSS Logical Properties where appropriate
Use CSS nesting where appropriate

Remaining tasks

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.

User interface changes

None. There should be no visual differences.

Comment	File	Size	Author
#37	interdiff-33_37.txt	1.26 KB	Gauravvvv
#37	3294006-37.patch	1.17 KB	Gauravvvv
#37	11.x: PHP 8.2 & MySQL 8 29,959 pass
#34	3294006 after patch.png	9.12 KB	Harish1688
#33	interdiff-29_33.txt	1.17 KB	Gauravvvv
#33	3294006-33.patch	1.92 KB	Gauravvvv
#33	11.x: PHP 8.2 & MySQL 8 29,842 pass
#29	3294006-29.patch	1.26 KB	_utsavsharma
#29	10.1.x: PHP 8.1 & MySQL 5.7 28,805 pass
#26	reroll_diff_24-26.txt	7.23 KB	Medha Kumari
#26	3294006.9.5.x-26.patch	637 bytes	Medha Kumari
#26	10.1.x: PHP 8.1 & MySQL 5.7 Custom Commands Failed
#24	reroll_diff_3294006.txt	2.66 KB	Sakthivel M
#24	3294006.9.5.x-24.patch	6.45 KB	Sakthivel M
#24	9.5.x: PHP 8.1 & MySQL 5.7 30,194 pass
#22	3294006.9.5.x.patch	6.02 KB	Aditya4478
#22	9.5.x: PHP 8.1 & MySQL 5.7 30,164 pass
#20	3294006-version10.0.x.patch	10.21 KB	Aditya4478
#20	10.0.x: PHP 8.1 & MySQL 5.7 28,510 pass
#19	3294006-version9.5.x.patch	5.67 KB	Aditya4478
#19	9.5.x: PHP 8.1 & MySQL 5.7 30,134 pass
#18	3294006-version9.5.x.patch	2.49 KB	Aditya4478
#18	9.5.x: PHP 8.1 & MySQL 5.7 30,134 pass
#17	3294006-9.5.x.patch	5.31 KB	Aditya4478
#17	9.5.x: PHP 8.1 & MySQL 5.7 30,063 pass
#16	3294006-10.0.x.patch	6.36 KB	Aditya4478
#16	10.0.x: PHP 8.1 & MySQL 5.7 28,576 pass
#14	3294006.9.5.x.patch	6.81 KB	Aditya4478
#14	9.5.x: PHP 8.1 & MySQL 5.7 Checkout Error
#12	3294006-10.0.x-12.patch	17.03 KB	shashwat-tiwari
#12	10.0.x: PHP 8.1 & MySQL 5.7 28,574 pass, 1 fail
#6	interdiff-4-6.txt	946 bytes	shashwat-tiwari
#6	3294006-9.5.x-6.patch	5.74 KB	shashwat-tiwari
#6	9.5.x: PHP 8.1 & MySQL 5.7 30,063 pass
#4	3294006-9.5.x-4.patch	5.74 KB	Munavijayalakshmi
#4	9.5.x: PHP 8.1 & MySQL 5.7 Custom Commands Failed
#2	3294006-9.5.x.patch	5.81 KB	Aditya4478
#2	9.5.x: PHP 8.1 & MySQL 5.7 Custom Commands Failed

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

3 July 2022 at 20:04

sasanikolic created an issue. See original summary.

Comment #2

Aditya4478 CreditAttribution: Aditya4478 commented 28 July 2022 at 06:11

Status:

Active

» Needs review

File	Size
3294006-9.5.x.patch	5.81 KB
9.5.x: PHP 8.1 & MySQL 5.7 Custom Commands Failed

Comment #3

Munavijayalakshmi CreditAttribution: Munavijayalakshmi at Valuebound for Valuebound commented 28 July 2022 at 07:09

Assigned:

Unassigned

» Munavijayalakshmi

File	Size
3294006-9.5.x-4.patch	5.74 KB
9.5.x: PHP 8.1 & MySQL 5.7 Custom Commands Failed

themes/claro/css/components/form--text.pcss.css
146:5 ✖ Expected no more than 0 empty lines selector-max-empty-lines
153:1 ✖ Unexpected empty line before closing brace block-closing-brace-empty-line-before

Comment #5

Munavijayalakshmi CreditAttribution: Munavijayalakshmi at Valuebound for Valuebound commented 28 July 2022 at 07:32

Assigned:

Munavijayalakshmi

» Unassigned

Comment #6

shashwat-tiwari CreditAttribution: shashwat-tiwari at Specbee for Drupal India Association commented 28 July 2022 at 11:50

File	Size
3294006-9.5.x-6.patch	5.74 KB
9.5.x: PHP 8.1 & MySQL 5.7 30,063 pass
interdiff-4-6.txt	946 bytes

Updated the patch for 9.5.x.

Comment #7

shashwat-tiwari CreditAttribution: shashwat-tiwari at Specbee for Drupal India Association commented 28 July 2022 at 11:53

Issue tags:

+Needs reroll

Patch needs to be rerolled for 10.0.x branch.

Comment #8

Munavijayalakshmi CreditAttribution: Munavijayalakshmi at Valuebound for Valuebound commented 28 July 2022 at 15:07

Version:	9.5.x-dev	» 10.0.x-dev
Assigned:	Unassigned	» Munavijayalakshmi
Status:	Needs review	» Needs work

Comment #9

Munavijayalakshmi CreditAttribution: Munavijayalakshmi at Valuebound for Valuebound commented 28 July 2022 at 15:44

Assigned:

Munavijayalakshmi

» Unassigned

Comment #10

Munavijayalakshmi CreditAttribution: Munavijayalakshmi at Valuebound for Valuebound commented 28 July 2022 at 15:48

hi @shashwat-tiwari,
How to rectified error in #4.
can u please explain steps.
i am new to Drupal contributions.

Comment #11

shashwat-tiwari CreditAttribution: shashwat-tiwari at Specbee for Drupal India Association commented 28 July 2022 at 16:17

hi @Munavijayalakshmi,

I followed the below steps:

Applied the patch given at #4.
Executed the shell command sh ./core/scripts/dev/commit-code-check.sh at root directory and validated the issue given at #4.
Made the changes in core/themes/claro/css/components/form--text.pcss.css file.
Executed the command yarn run build:css at core directory.
Once step 4 is successful, I executed the shell command mentioned in Step 2 again and verified the code build success.
Created the patch.
Removed the patch files after taking the backup and applied the patch given at #4.
Created a interdiff file for differences between the patch given at #4 and #6.

For 10.0.x, I'm creating a new patch because the existing patch is not getting applied for it.

Comment #12

shashwat-tiwari CreditAttribution: shashwat-tiwari at Specbee for Drupal India Association commented 28 July 2022 at 16:42

Status:

Needs work

» Needs review

File	Size
3294006-10.0.x-12.patch	17.03 KB
10.0.x: PHP 8.1 & MySQL 5.7 28,574 pass, 1 fail

Created the patch for 10.0.x.

Comment #13

28 July 2022 at 17:41

Status:

Needs review

» Needs work

The last submitted patch, 12: 3294006-10.0.x-12.patch, failed testing. View results

Comment #14

Aditya4478 CreditAttribution: Aditya4478 commented 29 July 2022 at 06:41

Version:	10.0.x-dev	» 9.5.x-dev
Status:	Needs work	» Needs review

File	Size
3294006.9.5.x.patch	6.81 KB
9.5.x: PHP 8.1 & MySQL 5.7 Checkout Error

5 files were hidden/shown/deleted

File	Size
3294006-9.5.x.patch	5.81 KB
9.5.x: PHP 8.1 & MySQL 5.7 Custom Commands Failed
3294006-9.5.x-4.patch	5.74 KB
9.5.x: PHP 8.1 & MySQL 5.7 Custom Commands Failed
3294006-9.5.x-6.patch	5.74 KB
9.5.x: PHP 8.1 & MySQL 5.7 30,063 pass
interdiff-4-6.txt	946 bytes
3294006-10.0.x-12.patch	17.03 KB
10.0.x: PHP 8.1 & MySQL 5.7 28,574 pass, 1 fail

Block for IE included. But there is some other error. I will post new patch soon

Comment #15

imalabya

he/him

English

Bangalore

CreditAttribution: imalabya at Specbee for Drupal India Association commented 29 July 2022 at 07:15

Status:

Needs review

» Needs work

@Aditya4478 When you work on a patch on top of a previous patch, it is a good practice to add an interdiff file which will help the maintaners to review the new changes.
https://www.drupal.org/docs/develop/git/using-git-to-contribute-to-drupa...

Comment #16

Aditya4478 CreditAttribution: Aditya4478 commented 30 July 2022 at 11:26

Version:

9.5.x-dev

» 10.0.x-dev

File	Size
3294006-10.0.x.patch	6.36 KB
10.0.x: PHP 8.1 & MySQL 5.7 28,576 pass

1 file was hidden/shown/deleted

File	Size
3294006.9.5.x.patch	6.81 KB
9.5.x: PHP 8.1 & MySQL 5.7 Checkout Error

ignore this patch

Comment #17

Aditya4478 CreditAttribution: Aditya4478 commented 30 July 2022 at 11:52

Version:	10.0.x-dev	» 9.5.x-dev
Status:	Needs work	» Needs review

File	Size
3294006-9.5.x.patch	5.31 KB
9.5.x: PHP 8.1 & MySQL 5.7 30,063 pass

1 file was hidden/shown/deleted

File	Size
3294006-10.0.x.patch	6.36 KB
10.0.x: PHP 8.1 & MySQL 5.7 28,576 pass

ignore this patch

Comment #18

Aditya4478 CreditAttribution: Aditya4478 commented 30 August 2022 at 14:05

File	Size
3294006-version9.5.x.patch	2.49 KB
9.5.x: PHP 8.1 & MySQL 5.7 30,134 pass

1 file was hidden/shown/deleted

File	Size
3294006-9.5.x.patch	5.31 KB
9.5.x: PHP 8.1 & MySQL 5.7 30,063 pass

wrong patch uploaded at wrong place. ignore this.

Comment #19

Aditya4478 CreditAttribution: Aditya4478 commented 30 August 2022 at 16:36

Version:

9.5.x-dev

» 10.0.x-dev

File	Size
3294006-version9.5.x.patch	5.67 KB
9.5.x: PHP 8.1 & MySQL 5.7 30,134 pass

1 file was hidden/shown/deleted

File	Size
3294006-version9.5.x.patch	2.49 KB
9.5.x: PHP 8.1 & MySQL 5.7 30,134 pass

This is a patch of 9.5.x.
Nesting is done.

Comment #20

Aditya4478 CreditAttribution: Aditya4478 commented 30 August 2022 at 17:10

File	Size
3294006-version10.0.x.patch	10.21 KB
10.0.x: PHP 8.1 & MySQL 5.7 28,510 pass

1 file was hidden/shown/deleted

File	Size
3294006-version9.5.x.patch	5.67 KB
9.5.x: PHP 8.1 & MySQL 5.7 30,134 pass

Comment #21

sasanikolic CreditAttribution: sasanikolic at Google Summer of Code commented 30 August 2022 at 21:13

Status:

Needs review

» Needs work

+++ b/core/themes/claro/css/components/form--text.css
@@ -151,19 +138,25 @@ _:-ms-fullscreen,
-.form-element--type-textarea.error + .ck-editor > .ck-editor__main {

The selector here is wrong. This should result in just form-element--type-textarea.error + .ck-editor > .ck-editor__main.

+++ b/core/themes/claro/css/components/form--text.css
@@ -66,23 +66,29 @@
+   * Reset value border and background of the file input on IE11 and Edge.

This results in an empty selector with a comment inside? This is weird...

+++ b/core/themes/claro/css/components/form--text.css
@@ -66,23 +66,29 @@
+   * Better upload button alignment for Chrome.

Same as above.

Comment #22

Aditya4478 CreditAttribution: Aditya4478 commented 31 August 2022 at 06:35

Version:	10.0.x-dev	» 9.5.x-dev
Status:	Needs work	» Needs review

File	Size
3294006.9.5.x.patch	6.02 KB
9.5.x: PHP 8.1 & MySQL 5.7 30,164 pass

1 file was hidden/shown/deleted

File	Size
3294006-version10.0.x.patch	10.21 KB
10.0.x: PHP 8.1 & MySQL 5.7 28,510 pass

changes done as per #21.

Comment #23

sasanikolic CreditAttribution: sasanikolic at Google Summer of Code commented 17 September 2022 at 09:35

Status:

Needs review

» Needs work

+++ b/core/themes/claro/css/components/form--text.css
@@ -66,23 +66,28 @@
+.form-element--type-file {
+  /**
+   * Reset value border and background of the file input on IE11 and Edge.

+++ b/core/themes/claro/css/components/form--text.pcss.css
@@ -55,18 +55,21 @@
+.form-element--type-file {
...
+  &::-ms-value {

I don't like these comments being compiled to the parent selector when nesting. I would suggest we switch those to inline comments - in that case they compile correctly next to the selector.

So that would be in pcss.css:

.form-element--type-file {
  &::-ms-value { /* ... */
    ... 
  }
}

I would like to get some other inputs in regards to that. Perhaps from @mherchel?

Comment #24

Sakthivel M CreditAttribution: Sakthivel M at QED42 for Drupal India Association commented 6 October 2022 at 08:03

Status:

Needs work

» Needs review

File	Size
3294006.9.5.x-24.patch	6.45 KB
9.5.x: PHP 8.1 & MySQL 5.7 30,194 pass
reroll_diff_3294006.txt	2.66 KB

#23 comments Fixed, Please verify the patch #24

Comment #25

6 October 2022 at 08:03

Version:

9.5.x-dev

» 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Comment #26

Medha Kumari

Jaipur

CreditAttribution: Medha Kumari as a volunteer and at Dotsquares Ltd. commented 12 October 2022 at 05:42

Issue tags:

-Needs reroll

File	Size
3294006.9.5.x-26.patch	637 bytes
10.1.x: PHP 8.1 & MySQL 5.7 Custom Commands Failed
reroll_diff_24-26.txt	7.23 KB

Rerolled patch #24 in 10.1.x .

Comment #27

ckrina

she/her

Catalan

Barcelona

CreditAttribution: ckrina at Lullabot commented 29 November 2022 at 16:41

Parent issue:

» #3324398: [META] Update Claro CSS with new coding standards

Comment #28

ckrina

she/her

Catalan

Barcelona

CreditAttribution: ckrina at Lullabot commented 29 November 2022 at 16:57

Issue summary:

View changes

Comment #29

_utsavsharma CreditAttribution: _utsavsharma at OpenSense Labs for DrupalFit commented 12 December 2022 at 09:54

File	Size
3294006-29.patch	1.26 KB
10.1.x: PHP 8.1 & MySQL 5.7 28,805 pass

Rerolled patch #24 in 10.1.x .
Please review.

Comment #30

smustgrave CreditAttribution: smustgrave at Mobomo commented 15 December 2022 at 14:24

Status:	Needs review	» Postponed
Issue tags:		+Needs Review Queue Initiative

This 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 #31

15 December 2022 at 14:24

Version:

10.1.x-dev

» 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Comment #32

Aditya4478 CreditAttribution: Aditya4478 commented 18 June 2023 at 04:36

Status:

Postponed

» Needs work

Comment #33

Gauravvvv CreditAttribution: Gauravvvv at Axelerant for Drupal India Association commented 29 June 2023 at 08:03

Status:

Needs work

» Needs review

File	Size
3294006-33.patch	1.92 KB
11.x: PHP 8.2 & MySQL 8 29,842 pass
interdiff-29_33.txt	1.17 KB

Updated logical properties in #29, attached interdiff for same. please review

Comment #34

Harish1688 CreditAttribution: Harish1688 at Material commented 29 June 2023 at 10:13

File	Size
3294006 after patch.png	9.12 KB

Hi,
After applying the patch 3294006-33.patch, the following points were observed:

1. CSS Logical Properties and nesting are utilized appropriately. (mostly all classes have variables, so found few changes on css Properties - like margin and text-indent)
2. The UI and focus states remain unchanged, showing no difference in behavior compared to the previous version.

Screenshot attached

looks good to RTBC

Comment #35

smustgrave CreditAttribution: smustgrave at Mobomo commented 29 June 2023 at 13:21

Status:

Needs review

» Reviewed & tested by the community

Comment #36

lauriii

he/him

Finnish

Finland

CreditAttribution: lauriii at Acquia commented 23 July 2023 at 18:06

Status:

Reviewed & tested by the community

» Needs review

+++ b/core/themes/claro/css/components/form--text.pcss.css
@@ -50,7 +50,7 @@
+  text-indent: calc(0.75rem - 0.0625rem); /* Text-input fallback for non-supporting browsers like Safari */

Why is this not using the variable anymore?

Comment #37

Gauravvvv CreditAttribution: Gauravvvv at Axelerant for Drupal India Association commented 24 July 2023 at 03:15

File	Size
3294006-37.patch	1.17 KB
11.x: PHP 8.2 & MySQL 8 29,959 pass
interdiff-33_37.txt	1.26 KB

Updated the variable, added the interdiff for same

Comment #38

smustgrave CreditAttribution: smustgrave at Mobomo commented 24 July 2023 at 17:11

Status:

Needs review

» Reviewed & tested by the community

Restoring to RTBC

Comment #39

1 August 2023 at 07:57

Status:

Reviewed & tested by the community

» Needs work

The last submitted patch, 37: 3294006-37.patch, failed testing. View results

Comment #40

Gauravvvv CreditAttribution: Gauravvvv at Axelerant for Drupal India Association commented 3 August 2023 at 03:19

Status:

Needs work

» Reviewed & tested by the community

Restoring to RTBC

Comment #41

7 August 2023 at 10:56

Status:

Reviewed & tested by the community

» Needs work

The last submitted patch, 37: 3294006-37.patch, failed testing. View results

Comment #42

Gauravvvv CreditAttribution: Gauravvvv at Axelerant for Drupal India Association commented 9 August 2023 at 03:28

Status:

Needs work

» Reviewed & tested by the community

Comment #43

16 August 2023 at 08:54

lauriii committed 0d20b5b4 on 11.x

Issue #3294006 by Aditya4478, Gauravvvv, shashwat-tiwari, Sakthivel M,...

Comment #44

lauriii

he/him

Finnish

Finland

CreditAttribution: lauriii at Acquia commented 16 August 2023 at 08:54

Status:	Reviewed & tested by the community	» Fixed
Issue tags:	-Needs followup

I don't think we need a follow-up here because inputs are spread across multiple files, and we can't really utilize component level custom properties for that reason.

Committed 0d20b5b and pushed to 11.x. Thanks!

Comment #45

30 August 2023 at 08:54

Status:

Fixed

» Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.