Problem/Motivation

See: #1995272: [Meta] Refactor module CSS files inline with our CSS standards

Proposed resolution

Review the CSS against our standards, see:
http://drupal.org/node/1887918#best-practices
https://www.drupal.org/node/2408617
#1190252: [573] Use csslint as a weapon to beat the crappy CSS out of Drupal core

Remaining tasks

Review current CSS
Write a patch to fix the suggestions
Run CSSlint against the new CSS

User interface changes

None

API changes

None

CommentFileSizeAuthor
#34 overtall-table-width.png488.79 KBhoemmawelt
#26 interdiff-clean-up-language-css-2485439-26.txt1 KBhog
#26 clean-up-language-css-2485439-26.patch891 byteshog
#24 table.mp4497.55 KBmanjit.singh
#21 after.png68.94 KBmanjit.singh
#21 before.png69.48 KBmanjit.singh
#18 interdiff-2485439-12-18.txt439 bytesrudraram
#18 clean-up-language-css-2485439-18.patch891 bytesrudraram
#12 content-language-1.png114.69 KBmanjit.singh
#12 content-language.png113.61 KBmanjit.singh
#12 clean-up-language-css-2485439-12.patch452 bytesmanjit.singh
#5 Screen Shot 2015-05-26 at 8.56.54 pm.png111.03 KBrudraram
#3 clean-up-language-css-2485439-3.patch1.13 KBleeotzu
language.admin_.css_.txt206 byteslewisnyman

Comments

rudraram’s picture

rudraram commented

Language content settings form has no classes in the markup.

<form action="/en/admin/config/regional/content-language" method="post" id="language-content-settings-form" accept-charset="UTF-8">

lewisnyman’s picture

lewisnyman
he/him
commented

Yeah we should be able to add one

leeotzu’s picture

leeotzu commented
StatusFileSize
new clean-up-language-css-2485439-3.patch1.13 KB

Please review the patch.

manjit.singh’s picture

manjit.singh
Primary language Hindi
Location Chandigarh
commented
Status: Active » Needs review
rudraram’s picture

rudraram commented
StatusFileSize
new Screen Shot 2015-05-26 at 8.56.54 pm.png111.03 KB

The above patch looks fine but the styles of language.admin.css are getting overwritten by content_translation.admin.css. Screenshot attached.

ss

rudraram’s picture

rudraram commented
Status: Needs review » Needs work
lewisnyman’s picture

lewisnyman
he/him
commented
Status: Needs work » Postponed
Related issues: +#2485409: Clean up content translation CSS inline with our CSS standards

Ok then, postponing on #2485409: Clean up content translation CSS inline with our CSS standards

manjit.singh’s picture

manjit.singh
Primary language Hindi
Location Chandigarh
commented

@rudraram #5 Can you specify the URL, where you are tested and screenshots belongs to ?

rudraram’s picture

rudraram commented

@Manjit the url is admin/config/regional/content-language

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Postponed » Needs work

The blocker has been committed - we can now do this one.

manjit.singh’s picture

manjit.singh
Primary language Hindi
Location Chandigarh
commented
Assigned: Unassigned » manjit.singh
manjit.singh’s picture

manjit.singh
Primary language Hindi
Location Chandigarh
commented
Assigned: manjit.singh » Unassigned
Status: Needs work » Needs review
StatusFileSize
new clean-up-language-css-2485439-12.patch452 bytes
new content-language.png113.61 KB
new content-language-1.png114.69 KB

I think we don't need to add class now. Class is added in this issue #2485409: Clean up content translation CSS inline with our CSS standards. And css is already there in content_translation.admin.css. I have removed CSS file (language.admin.css) in patch. Please check screenshot as well.

Check there is no override in CSS

alt

alt

Status: Needs review » Needs work

The last submitted patch, 12: clean-up-language-css-2485439-12.patch, failed testing.

The last submitted patch, 12: clean-up-language-css-2485439-12.patch, failed testing.

rudraram’s picture

rudraram commented

@Manjit.Singh I think you should also remove the language.admin.css from the language.libraries.yml file too.

manjit.singh’s picture

manjit.singh
Primary language Hindi
Location Chandigarh
commented

yup that's what i was thinking about, can you take this up please

rudraram’s picture

rudraram commented

Sure I'll work on this.

rudraram’s picture

rudraram commented
Status: Needs work » Needs review
StatusFileSize
new clean-up-language-css-2485439-18.patch891 bytes
new interdiff-2485439-12-18.txt439 bytes

Removed language.admin.css from the language.libraries.yml. Attaching patch and interdiff,

rudraram’s picture

rudraram commented

Now we need some screenshots on this. @Manjit can you add some screenshots just like you did in #12

manjit.singh’s picture

manjit.singh
Primary language Hindi
Location Chandigarh
commented
Assigned: Unassigned » manjit.singh
manjit.singh’s picture

manjit.singh
Primary language Hindi
Location Chandigarh
commented
StatusFileSize
new before.png69.48 KB
new after.png68.94 KB

@rudraram there is some minor difference in the table. Please check the screenshots.

Before
alt

After
alt

manjit.singh’s picture

manjit.singh
Primary language Hindi
Location Chandigarh
commented
Assigned: manjit.singh » Unassigned
Status: Needs review » Needs work
rudraram’s picture

rudraram commented

@manjit are you talking about the size of the "content type" column?

Sorry I'm not able to exactly see the difference. It would be very helpful if you could mark the difference.

manjit.singh’s picture

manjit.singh
Primary language Hindi
Location Chandigarh
commented
StatusFileSize
new table.mp4497.55 KB

@rudraram check table differences in screencast :)

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

hog’s picture

hog commented
Status: Needs work » Needs review
StatusFileSize
new clean-up-language-css-2485439-26.patch891 bytes
new interdiff-clean-up-language-css-2485439-26.txt1 KB

Re-rolled patch.
No regressions now for me.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs review » Reviewed & tested by the community

I think that small visual diff (#21 #24) is fine

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work

We don't remove CSS like this - the core promise is to move these things to Stable so theme's extending form classy and stable don't change but core's markup and css can improve.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

hoemmawelt’s picture

hoemmawelt commented
StatusFileSize
new overtall-table-width.png488.79 KB

I'm a little bit confused - deleting the width on both classes will break the layout of the entire table. The length of content values will than determine table row width.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
Issue summary: View changes
Status: Needs work » Closed (works as designed)

Yup, that sounds reasonable to me. Closing. Please re-open if there is anything to do here.