Problem/Motivation

Currently Views has a specific style sheet for each core theme. Some of the styles are needed for Views, and others duplicate styles that are already produced by the theme.

Proposed resolution

Merge the Views custom styles into to the actual core themes.

Remaining tasks

  • Each Views custom theme sheet should be added to its respective theme's CSS
  • After styles have been included with their theme, remove the custom sheets from Views
  • Refactor the added Views styles accordingly, with special attention paid to style duplication

User interface changes

n/a

API changes

n/a

CommentFileSizeAuthor
#9 vdc-1826574-9.patch24.21 KBtim.plunkett
#9 interdiff.txt530 bytestim.plunkett
#8 vdc-1826574-6-formatpatch.patch29.78 KBdead_arm
#8 vdc-1826574-6.patch24.1 KBdead_arm
#6 durpal-1826574-6.patch30.23 KBdawehner
#4 drupal-1826574-4.patch30.24 KBdawehner
#4 interdiff.txt1.77 KBdawehner
#3 vdc-1826574-3.patch28.47 KBdead_arm
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dead_arm’s picture

dead_arm
she/her
Primary language English
CreditAttribution: dead_arm commented
Assigned: Unassigned » dead_arm

Working on an issue summary.

yoroy’s picture

yoroy CreditAttribution: yoroy commented

This will help resolve #1806022: Views' text color does not have sufficient contrast which is a critical. Go for it :)

dead_arm’s picture

dead_arm
she/her
Primary language English
CreditAttribution: dead_arm commented
Status: Active » Needs review
FileSize
vdc-1826574-3.patch28.47 KB

Here's a preliminary patch. Leaving assigned to myself to remove further redundancies.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
FileSize
interdiff.txt1.77 KB
drupal-1826574-4.patch30.24 KB

Great work!
We could remove some php code if we move the css files, awesome.

Status: Needs review » Needs work

The last submitted patch, drupal-1826574-4.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs work » Needs review
FileSize
durpal-1826574-6.patch30.23 KB

Rerolled against tim :p

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented 
+++ b/core/modules/views/views_ui/lib/Drupal/views_ui/ViewFormControllerBase.phpundefined
@@ -66,22 +66,15 @@ public static function getAdminCSS() {
     $theme_key = $GLOBALS['theme'];
     while ($theme_key) {
       // Try to find the admin css file for non-core themes.
-      if (!in_array($theme_key, array('seven', 'bartik'))) {
-        $theme_path = drupal_get_path('theme', $theme_key);
-        // First search in the css directory, then in the root folder of the theme.
-        if (file_exists($theme_path . "/css/views-admin.$theme_key.css")) {
-          $list[$theme_path . "/css/views-admin.$theme_key.css"] = array(
-            'group' => CSS_THEME,
-          );
-        }
-        elseif (file_exists($theme_path . "/views-admin.$theme_key.css")) {
-          $list[$theme_path . "/views-admin.$theme_key.css"] = array(
-            'group' => CSS_THEME,
-          );
-        }
+      $theme_path = drupal_get_path('theme', $theme_key);
+      // First search in the css directory, then in the root folder of the theme.
+      if (file_exists($theme_path . "/css/views-admin.$theme_key.css")) {
+        $list[$theme_path . "/css/views-admin.$theme_key.css"] = array(
+          'group' => CSS_THEME,
+        );
       }
-      else {
-        $list[$module_path . "/css/views-admin.$theme_key.css"] = array(
+      elseif (file_exists($theme_path . "/views-admin.$theme_key.css")) {
+        $list[$theme_path . "/views-admin.$theme_key.css"] = array(
           'group' => CSS_THEME,
         );

All of this code should go away; there is no reason we should assume that non-core themes are used.

dead_arm’s picture

dead_arm
she/her
Primary language English
CreditAttribution: dead_arm commented
Assigned: dead_arm » Unassigned
FileSize
vdc-1826574-6.patch24.1 KB
vdc-1826574-6-formatpatch.patch29.78 KB

Okay, here's the whole thing, one as a format patch for readability, and one as a regular patch.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Priority: Normal » Critical
Status: Needs review » Reviewed & tested by the community
FileSize
interdiff.txt530 bytes
vdc-1826574-9.patch24.21 KB

This is holding up a critical (#1806022: Views' text color does not have sufficient contrast), so it shouldn't be a normal task.

There were only two visual changes lost in that patch, I've added them back. Otherwise, as this was fully manually tested, it's ready to go.

Status: Reviewed & tested by the community » Needs work
Issue tags: -VDC

The last submitted patch, vdc-1826574-9.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs work » Needs review

#9: vdc-1826574-9.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +VDC

The last submitted patch, vdc-1826574-9.patch, failed testing.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Needs work » Fixed

That's definitely a bit silly. Also, CSS patches don't tend to fail Drupal installation. ;)

Committed to 8.x. Will rebase and push when I'm done with the current commit spree.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Assigned: Unassigned » tim.plunkett
Priority: Critical » Normal
Status: Fixed » Needs work

Apparently this wasn't fully tested with the changes in #723392: Tame seven's reset.css, and effectively conflicted.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Assigned: tim.plunkett » Unassigned
Priority: Normal » Critical
Status: Needs work » Fixed

Apparently this was fixed somewhere else, I can no longer reproduce.

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

Anonymous’s picture

(not verified) CreditAttribution: commented
Issue summary: View changes

Update issue summary.