Problem

Expected: A list item in a filter tip is rendered with normal margins.

Actual: The forms.css stylesheet from the administration theme (Seven) adds a margin to the inline list items, which renders them very far apart.

The problem can be traced to this CSS rule:

ul.tips li {
  margin: 0.25em 0 0.25em 1.5em; /* LTR */
}
[dir="rtl"] ul.tips li {
  margin: 0.25em 1.5em 0.25em 0;
}

Example:
bad list margins screenshot

Reproduction

Simplest is to navigate to /filter/tips on a standard install with the 7 theme set as default.

Solution

The correct selector to use here would be ul.tips > li, to apply the style only to the actual tips list, rather than lists nested inside it.
Fixed:
fixed list margins screenshot

CommentFileSizeAuthor
#35 interdiff_28-35.txt402 bytesravi.shankar
#35 2896949-35.patch838 bytesravi.shankar
#28 2896949-28.patch836 bytesmanojkumar_97
#28 After-patch2.jpeg118.41 KBmanojkumar_97
#28 After-patch.jpeg97.89 KBmanojkumar_97
#19 2896949-bartik-after-patch-19.png20.7 KBIndrajithKB
#19 2896949-after-patch-18-bartik.png19.68 KBIndrajithKB
#19 interdiff_18-19.txt288 bytesIndrajithKB
#19 2896949-19.patch4.08 KBIndrajithKB
#18 2896949-18.patch3.69 KBcburschka
#18 interdiff-16-18.txt2.15 KBcburschka
#16 2896949-after-patch-16.png96.96 KBIndrajithKB
#16 2896949-after-patch-11.png101.6 KBIndrajithKB
#16 interdiff_11-16.txt1.04 KBIndrajithKB
#16 2896949-16.patch1.54 KBIndrajithKB
#15 filter-tips-2.png13.06 KBcburschka
#15 filter-tips-1.png13.98 KBcburschka
#11 2896949-11.patch1020 byteslauriii
#8 composer-tips-good.png11.82 KBNickDickinsonWilde
#8 composer-tips-bad.png12.22 KBNickDickinsonWilde
#2 drupal-2896949-tips-list-style.patch494 bytescburschka

Issue fork drupal-2896949

Command icon Show commands

Start 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:

About issue forks
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

cburschka created an issue. See original summary.

cburschka’s picture

cburschka
they
CreditAttribution: cburschka commented
Version: 8.3.x-dev » 8.4.x-dev
Status: Active » Needs review
FileSize
drupal-2896949-tips-list-style.patch494 bytes

Patch for both 8.4.x and 8.3.x.

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

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now 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.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now 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.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now 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.

NickDickinsonWilde’s picture

NickDickinsonWilde
He/Him/His
Primary language English
Location Victoria, BC (T'So-uke lands)
CreditAttribution: NickDickinsonWilde as a volunteer and commented
Status: Needs review » Reviewed & tested by the community

Patches still apply.
Doesn't need tests and only affects the default seven admin theme so despite the css change and is very specific, I believe this can be pulled back to 8.6.X as well.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Acquia commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs steps to reproduce, +Needs screenshots

Could you add a full list of steps needed to reproduce the bug in the issue summary?

Also, since the patch makes visual changes, would be useful to have a screenshot added to the issue summary.

NickDickinsonWilde’s picture

NickDickinsonWilde
He/Him/His
Primary language English
Location Victoria, BC (T'So-uke lands)
CreditAttribution: NickDickinsonWilde as a volunteer and commented
Issue summary: View changes
Status: Needs work » Needs review
Issue tags: -Needs steps to reproduce, -Needs screenshots
FileSize
composer-tips-bad.png12.22 KB
composer-tips-good.png11.82 KB

Updated summary with further details.

gnuschichten’s picture

gnuschichten CreditAttribution: gnuschichten at ETECTURE GmbH commented

Patch still applies on 8.7.x-dev and works fine.

gnuschichten’s picture

gnuschichten CreditAttribution: gnuschichten at ETECTURE GmbH commented
Status: Needs review » Reviewed & tested by the community
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Acquia commented
Status: Reviewed & tested by the community » Needs review
FileSize
2896949-11.patch1020 bytes

Another approach would be to add a class for the specific element we want to add the style for. The downside of this approach is that it requires either a template override in Seven or template change in Classy. I went ahead with changing the template in Classy but we might want to instead override the template in Seven.

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

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.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.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). 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.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now 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.

cburschka’s picture

cburschka
they
CreditAttribution: cburschka commented
FileSize
filter-tips-1.png13.98 KB
filter-tips-2.png13.06 KB

These screenshots show the problem more distinctly than the ones in the top post.

filter-tips-bad

filter-tips-good

IndrajithKB’s picture

IndrajithKB CreditAttribution: IndrajithKB at Srijan | A Material+ Company for Drupal India Association commented
FileSize
2896949-16.patch1.54 KB
interdiff_11-16.txt1.04 KB
2896949-after-patch-11.png101.6 KB
2896949-after-patch-16.png96.96 KB

Hi @lauriii Thanks for the #11 patch.
Here i override the seven template.

attaching the screenshot after applying#11
Only local images are allowed.

attaching the screenshot after applying #16
patch16

please review.

Status: Needs review » Needs work

The last submitted patch, 16: 2896949-16.patch, failed testing. View results

cburschka’s picture

cburschka
they
CreditAttribution: cburschka commented
Status: Needs work » Needs review
FileSize
interdiff-16-18.txt2.15 KB
2896949-18.patch3.69 KB

Changes to the classy templates must also be applied to the other themes' copies and the respective file hash in ConfirmClassyCopyTest.

IndrajithKB’s picture

IndrajithKB CreditAttribution: IndrajithKB at Srijan | A Material+ Company for Drupal India Association commented
FileSize
2896949-19.patch4.08 KB
interdiff_18-19.txt288 bytes
2896949-after-patch-18-bartik.png19.68 KB
2896949-bartik-after-patch-19.png20.7 KB

Hi @cburschka thanks for the #18 patch,
It's working fine. But i faced one issue with bartik theme after applying your patch #18

screenshot of bartik after applied #18:

bartik

i have fixed the issue and the screenshot is:

bartik

priyanka.sahni’s picture

priyanka.sahni CreditAttribution: priyanka.sahni at Srijan | A Material+ Company for Drupal India Association commented
Assigned: Unassigned » priyanka.sahni
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Acquia commented
Status: Needs review » Needs work 
+++ b/core/tests/Drupal/KernelTests/Core/Theme/ConfirmClassyCopiesTest.php
@@ -743,7 +743,7 @@ protected function getClassyHash($type, $file) {
-        'filter-tips.html.twig' => 'fefcab317b602cbfe7f608bc481be889',
+        'filter-tips.html.twig' => 'ae432c3f23ed000af85e89d0c77562af',

Instead of updating this, we could move this file to the templates directory in Bartik and Seven where we would like to leverage .tips__item. This way we don't have to update the hash and templates in all themes.

priyanka.sahni’s picture

priyanka.sahni CreditAttribution: priyanka.sahni at Srijan | A Material+ Company for Drupal India Association commented
Assigned: priyanka.sahni » Unassigned

Un assigning the issue as the status changed to Needs Work , earlier I had assigned to test the issue.

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

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

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

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

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

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented
Issue tags: +Novice, +Bug Smash Initiative
manojkumar_97’s picture

manojkumar_97 CreditAttribution: manojkumar_97 at OpenSense Labs commented
Assigned: Unassigned » manojkumar_97
manojkumar_97’s picture

manojkumar_97 CreditAttribution: manojkumar_97 at OpenSense Labs commented
Assigned: manojkumar_97 » Unassigned
Status: Needs work » Reviewed & tested by the community
FileSize
After-patch.jpeg97.89 KB
After-patch2.jpeg118.41 KB
2896949-28.patch836 bytes

After apply patch:

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Acquia commented
Status: Reviewed & tested by the community » Needs work

CI is not passing for the latest patch.

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

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now 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.

rpayanm made their first commit to this issue’s fork.

rpayanm opened merge request !2324

rpayanm’s picture

rpayanm
he/him
Primary language Spanish
CreditAttribution: rpayanm commented
Status: Needs work » Needs review
Shubham Sharma 77’s picture

Shubham Sharma 77 CreditAttribution: Shubham Sharma 77 at Srijan | A Material+ Company for Drupal India Association commented
Status: Needs review » Needs work

Status changed to Needs Work. The last patch failed

ravi.shankar’s picture

ravi.shankar CreditAttribution: ravi.shankar at OpenSense Labs commented
FileSize
2896949-35.patch838 bytes
interdiff_28-35.txt402 bytes

Just fixed the Drupal CS issue of patch #28.

rpayanm’s picture

rpayanm
he/him
Primary language Spanish
CreditAttribution: rpayanm commented
Status: Needs work » Needs review
smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented
Status: Needs review » Reviewed & tested by the community

selectors look valid.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented
Status: Reviewed & tested by the community » Postponed

We're actively trying to deprecate the Seven theme, so postponing this in the meantime.
Once we move it to contrib, we may choose to unpostpone this issue.

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Project: Drupal core » Seven
Version: 9.5.x-dev » 1.0.0-alpha1
Component: Seven theme » Code
Status: Postponed » Reviewed & tested by the community

The Seven theme has been removed from Drupal 10 core. I confirmed that this issue only affects Seven and no other themes included with Drupal core, so I am moving this to the contributed Seven project.