Problem/Motivation

There is quite a lot of CSS in the Views UI that is not being used, or CSS that is overriding other CSS that it shouldn't.

Remaining tasks

Review the CSS for Views UI that we can safely remove without any regressions.
Remove them.

User interface changes

None

API changes

None

Views testing process

Views is by far Drupal Core's most complex UI, it requires it's own testing coverage to ensure a commit does not cause regressions. Each issue can provide before/after screenshots of the following use cases.

Simple use cases

  • Add a new view via the wizard (admin/structure/views/add
  • Select the basics and hit save.
  • On the view edit form, select a new filter for the node type
  • Change the row format to show fields
  • Add the body field to the output, and rewrite the output to use
    [body]
  • save the view

Advanced use cases

  • Enable a display
  • Disable a display
  • Choose the table style and enable click sort
  • Add a filter by title
  • Add a filter by author
  • Rearrange the filters to have the author first
  • Add a relationship
  • Override the fields configuration // To test the override select
  • Add a contextual filter for Node: ID and setup a validation for a specific node: type
  • Expose a filter
  • Click preview

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Task because coding standards
Issue priority Not critical because coding standards
Unfrozen changes Unfrozen because it only changes markup
CommentFileSizeAuthor
#33 2539386-33.patch20.17 KBameymudras
#19 removed_unused_css_from-2539386-19.patch22.46 KBsvendecabooter
#16 removed_unused_css_from-2539386-15.patch21.81 KBmathieuspil
#15 interdiff-2539386-14-15.txt431 bytesmathieuspil
#14 removed_unused_css_from-2539386-14.patch21.59 KBmathieuspil
#6 screenshot-views-ui-admin-bug.png74.47 KBAleksandar_P
#6 removed_unused_css_from_views_ui_admin-2539386-6.patch8.37 KBAleksandar_P
#3 removed_unused_css_from-2539386-3.patch21.54 KBlewisnyman
#3 interdiff.txt1.35 KBlewisnyman
#1 removed_unused_css_from-2539386-1.patch20.41 KBlewisnyman

Comments

lewisnyman’s picture

lewisnyman
he/him
commented
Status: Active » Needs review
StatusFileSize
new removed_unused_css_from-2539386-1.patch20.41 KB

WIP - Have at it testbot

Status: Needs review » Needs work

The last submitted patch, 1: removed_unused_css_from-2539386-1.patch, failed testing.

lewisnyman’s picture

lewisnyman
he/him
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.txt1.35 KB
new removed_unused_css_from-2539386-3.patch21.54 KB

Fixing tests

Status: Needs review » Needs work

The last submitted patch, 3: removed_unused_css_from-2539386-3.patch, failed testing.

Aleksandar_P’s picture

Aleksandar_P commented
Assigned: Unassigned » Aleksandar_P
Aleksandar_P’s picture

Aleksandar_P commented
StatusFileSize
new removed_unused_css_from_views_ui_admin-2539386-6.patch8.37 KB
new screenshot-views-ui-admin-bug.png74.47 KB

Cleaned unused css from views-ui.

However, I noticed that 'more' options tab in 'Configure filter criterion: Content: Type' is overlapping the floated elements above it. I tried using `clear: both` on that element and it solved that problem but have not included it in this patch. Is there any suggestion on fixing it some other way?

Aleksandar_P’s picture

Aleksandar_P commented
Assigned: Aleksandar_P » Unassigned
Status: Needs work » Needs review
lewisnyman’s picture

lewisnyman
he/him
commented

@Aleksandar_P Is the patch in #6 related to the patch in #3? I'm a little confused because the file size is different.

Aleksandar_P’s picture

Aleksandar_P commented

@LewisNyman I am not sure what do you mean by related. I did not use any of the patches since they failed the simple test. That's why I haven't created the interdiff.

lewisnyman’s picture

lewisnyman
he/him
commented

@Aleksandar_P If you to build on top of previous patches and fix the tests you don't risk losing previous work.

Aleksandar_P’s picture

Aleksandar_P commented

@LewisNyman Was I supposed to work on the patch you previously uploaded?

mathieuspil’s picture

mathieuspil commented
Assigned: Unassigned » mathieuspil
Status: Needs review » Needs work

@Aleksandar_P If a patch fails, we should look into how that patch fails and fix that.

Will start of from #3 and see what #6 brings to the table.

Aleksandar_P’s picture

Aleksandar_P commented

@MathieuSpil Oh, ok. I could not clearly understand and was not aware of that. I will be following this issue. Good luck.

mathieuspil’s picture

mathieuspil commented
StatusFileSize
new removed_unused_css_from-2539386-14.patch21.59 KB

Recreated the patch from #3 so it applies again, won't be adding the interdiff, because that is very confusing :)

Next step: do the things I was going to do in #12

mathieuspil’s picture

mathieuspil commented
Assigned: mathieuspil » Unassigned
Status: Needs work » Needs review
StatusFileSize
new interdiff-2539386-14-15.txt431 bytes

So in #6 I saw a couple of things
In views_ui.admin.theme.css:

.views-admin .icon {
-  background-attachment: scroll;
    background-image: url(../images/sprites.png);
-  background-position: left top; /* LTR */
-  background-repeat: no-repeat;
}

=> @LewisNyman: can you point me to another location where the .icon-class is being used inside views? (If it is only used once, this can be deleted indeed.)

-[dir="rtl"] .views-admin span.icon {
-  float: right;
-}

=> Floating right or the icon is needed for floating right in the rtl-language, so we shouldn't remove this code

@Aleksandar_P: I can still see the .form-item-live-previewin inspector, but in #6 you removed this selector form css? This causes some visual regression for this item

I also see that you placed some code in comments:

-.views-ui-display-tab-bucket .dropbutton-wrapper .dropbutton-widget .dropbutton-action a {
-  width: auto;
-}
+/*.views-ui-display-tab-bucket .dropbutton-wrapper .dropbutton-widget .dropbutton-action a {*/
+  /*width: auto;*/
+/*}*/

=> You should consider a patch as a suggestion you make for the codebase, so you may just remove the code inside a patch instead of placing it in comments.

Extra: for me it is very confusing to combine 2 patches, especially because they are both very big.

Patch

mathieuspil’s picture

mathieuspil commented
StatusFileSize
new removed_unused_css_from-2539386-15.patch21.81 KB

Drupalcon Internetz doesn't allow my patch-file :)

Status: Needs review » Needs work

The last submitted patch, 16: removed_unused_css_from-2539386-15.patch, failed testing.

The last submitted patch, 16: removed_unused_css_from-2539386-15.patch, failed testing.

svendecabooter’s picture

svendecabooter
he/him
Primary language Dutch
Location Gent
commented
Status: Needs work » Needs review
StatusFileSize
new removed_unused_css_from-2539386-19.patch22.46 KB

This seems to fix the failing test

lewisnyman’s picture

lewisnyman
he/him
commented

@LewisNyman: can you point me to another location where the .icon-class is being used inside views? (If it is only used once, this can be deleted indeed.)

The icon classes are used by the buttons, before they are converted into dropbuttons in Javascript. If you turn off Javascript then you can see them. I think we can remove them.

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.

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.

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

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

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

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

smustgrave’s picture

smustgrave commented
Version: 9.4.x-dev » 10.1.x-dev
Status: Needs review » Needs work

Patch or MR doesn't apply anymore
The last patch or MR doesn't apply to the target branch, please reroll the code so that it can be reviewed by the automated testbot.

ameymudras’s picture

ameymudras commented
Status: Needs work » Needs review
StatusFileSize
new 2539386-33.patch20.17 KB

Re rolled the patch #19 to D10.1.x

lendude’s picture

lendude
Primary language Dutch
Location Amsterdam
commented
Status: Needs review » Needs work

Nice to see a lot of CSS removed, but before we move on this, I really feel that we need a visual regression test. I know we can't do one on the testbot but running one locally and uploading the results would be really good here. Using something like backstopJs for this shouldn't be too hard to do.

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.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.