Make WordPress Core

Opened 7 months ago

Closed 23 hours ago

Last modified 23 hours ago

#60197 closed defect (bug) (fixed)

Twenty Fifteen: List block padding with a background color

Reported by: viralsampat's profile viralsampat Owned by: karmatosed's profile karmatosed
Milestone: 6.7 Priority: low
Severity: normal Version:
Component: Bundled Theme Keywords: has-patch commit
Focuses: css Cc:

Description

I have reviewed the "list block" into the "Twenty Fifteen" theme and found that its UL list type is not displaying properly when we add the list background color.

Backend:
Issue: https://share.cleanshot.com/V3MPvcVPWT0xDLn6Wy2H

Front-end
https://share.cleanshot.com/dtmrQKGqnR6wbKPvkrTt

Thanks,

Attachments (8)

60197.patch (459 bytes) - added by viralsampat 7 months ago.
I have checked above mentioned issue and resolved it. Here, I have added my patch.
CleanShot 2024-01-05 at 13.59.31@2x.png (94.3 KB) - added by viralsampat 7 months ago.
CleanShot 2024-01-05 at 13.59.31@2x.2.png (94.3 KB) - added by viralsampat 7 months ago.
Resolved
60197.2.patch (513 bytes) - added by sabernhardt 6 months ago.
twenty-fifteen-vs-twenty-twenty-four.jpg (113.2 KB) - added by deepakvijayan 6 months ago.
SCR-20240714-ihtn.png (152.3 KB) - added by karmatosed 7 days ago.
SCR-20240714-ihwg.png (46.8 KB) - added by karmatosed 7 days ago.
60197.nonframed.patch (493 bytes) - added by sabernhardt 42 hours ago.
adds padding to List blocks in the non-framed editor

Download all attachments as: .zip

Change History (23)

@viralsampat
7 months ago

I have checked above mentioned issue and resolved it. Here, I have added my patch.

#1 @devtanbir
6 months ago

@viralsampat I have tested your patch. But it's now working for me. See my example below:

editor-blocks.css

.wp-block-post-content ol.has-background,
.wp-block-post-content ul.has-background {
     padding: 1.25em 2.375em;
}

If I add like this, then it's work.

Last edited 6 months ago by devtanbir (previous) (diff)

#2 @sabernhardt
6 months ago

  • Component changed from Themes to Bundled Theme
  • Keywords has-patch added

I used .editor-styles-wrapper ul.has-background and .editor-styles-wrapper ol.has-background to override the zero padding on .edit-post-visual-editor ul:not(.wp-block-gallery) and .editor-styles-wrapper ol in editor-blocks.css.

#3 @deepakvijayan
6 months ago

@sabernhardt Not sure overriding .has-background would be the right way to go.

Since I can see this issue even when there is no background. I am attaching screenshots from themes "Twenty Fifteen" and "Twenty Twenty-Four" with no background colour selected for reference.

Let us keep it consistent as to what it's been currently used in "Twenty Twenty-Four".

#4 follow-up: @sabernhardt
6 months ago

Twenty Fifteen needs to remain consistent with the design decisions made for that theme nine years ago. People who prefer Twenty Twenty-Four can switch themes.

When no background color is defined, the lists are supposed to be in a "hanging punctuation" style (see #30374).

#5 in reply to: ↑ 4 @deepakvijayan
6 months ago

Replying to sabernhardt:

Twenty Fifteen needs to remain consistent with the design decisions made for that theme nine years ago. People who prefer Twenty Twenty-Four can switch themes.

When no background color is defined, the lists are supposed to be in a "hanging punctuation" style (see #30374).

I see. Adjusting the padding while .has-background is active would be the only option to go about it. I apologize for any confusion caused by my previous comment. Thank you for your hard work.

#6 @karmatosed
7 days ago

  • Keywords needs-testing added
  • Milestone changed from Awaiting Review to 6.7

Assigning to myself for testing to see if does fit the design decisions.

#7 @karmatosed
7 days ago

  • Milestone changed from 6.7 to Awaiting Review

Unless I am mistaken this is no longer an issue in testing. I might very well be wrong though. I am attaching what I see without applying the patch though in Safari and Chrome.

What I see with the latest patch is no change.

As a result of not seeing the issue currently, I am going to recommend this is closed, if we can get steps to replicate I absolutely will continue on with this though. Thank you everyone.

#8 @karmatosed
7 days ago

  • Status changed from new to assigned

#9 @karmatosed
7 days ago

  • Keywords close added

#10 @sabernhardt
7 days ago

  • Keywords 2nd-opinion added
  • Summary changed from Twenty Fifteen: List block list alignment issue. to Twenty Fifteen: List block padding with a background color

The iframe editor shows the padding correctly, but the non-framed editor sets the padding to 0:

  • The post editor assigns zero padding with .edit-post-visual-editor ul:not(.wp-block-gallery) in editor-blocks.css.
  • The widgets editor assigns zero padding with .editor-styles-wrapper ul, from the inline styles built on editor-style.css.

#11 @karmatosed
6 days ago

  • Keywords close removed

@sabernhardt I am going to remove the 'closed' whilst this is being discussed as it seems that is best here. I admit what I am seeing isn't the issue but as you note this is likely due to the editor framing. What would be your recommendation here?

@sabernhardt
42 hours ago

adds padding to List blocks in the non-framed editor

#12 @sabernhardt
41 hours ago

  • Keywords needs-testing removed
  • Priority changed from normal to low

The padding value for lists with a background comes from block-library styles, not the theme. If that amount ever changes, it would be inaccurate (but likely better than zero).

Maybe any change on this ticket should not affect the iframe editor. 60197.nonframed.patch targets only the List block (ordered or unordered), only in the non-framed editor.

div.editor-styles-wrapper .wp-block-list:where(.has-background) {
	padding: 1.25em 2.375em;
}

Another selector could do the same for the List and Latest Posts blocks:

div.editor-styles-wrapper .has-background:where(.wp-block-list, .wp-block-latest-posts)

60197.2.patch is simpler. It could also correct the left padding for a Latest Posts block with a background, whether or not the editor is framed, but the block-library styles could use :not(.has-background) for any theme (GB62830).

Last edited 41 hours ago by sabernhardt (previous) (diff)

#13 @karmatosed
23 hours ago

  • Keywords commit added; 2nd-opinion removed

I agree for now about getting the simpler patch in @sabernhardt. We can always iterate if need be but it does bring an option that is easier. Thank you, I will move towards to commit.

#14 @karmatosed
23 hours ago

  • Owner set to karmatosed
  • Resolution set to fixed
  • Status changed from assigned to closed

In 58772:

Twenty Fifteen: Fixes List Block with padding not having background color.

The List Block when had padding was not displaying the background color correctly. This only impacts the non-framed editor.

Props viralsampat, devtanbir, sabernhardt, deepakvijayan.
Fixes #60197.

#15 @karmatosed
23 hours ago

  • Milestone changed from Awaiting Review to 6.7

I added in the one that only impacts the non-framed editor to be clear. We can always take things from there but it feels like a good starting point and moves this ticket on. Thank you everyone.

Note: See TracTickets for help on using tickets.