WordPress.org

Make WordPress Core

Opened 3 months ago

Closed 3 weeks ago

#51157 closed defect (bug) (fixed)

Bundled Themes: List block appears too far to the left after 5.5 upgrade

Reported by: kjellr Owned by: sabernhardt
Milestone: 5.6 Priority: normal
Severity: normal Version:
Component: Bundled Theme Keywords: has-screenshots needs-testing dev-feedback has-patch
Focuses: css, rtl Cc:

Description (last modified by SergeyBiryukov)

After upgrading to WordPress v5.5, the list block appears too far to the left in the editor in some themes:

  • Twenty Fourteen
  • Twenty Sixteen
  • Twenty Seventeen

(Possibly a couple more, but we should test and verify)

This was fixed in Twenty Fifteen earlier this year, so these themes likely need a similar fix: #50029

More details in this GitHub issue:
https://github.com/WordPress/gutenberg/issues/24780


http://cldup.com/LkCxBQaH8Q.jpg

http://cldup.com/cG4MIyW3wJ.png

Attachments (20)

51157.patch (2.7 KB) - added by amolv 2 months ago.
Default List margin removed from theme.
Capture d’écran 2020-09-19 à 10.28.58.png (157.4 KB) - added by audrasjb 2 months ago.
After the patch (Twenty Eleven example)
Twenty-Fifteen-RTL-first-patch.png (151.5 KB) - added by sabernhardt 2 months ago.
Editor with various lists in Twenty Fifteen, in Arabic
Twenty-Eleven-RTL-first-patch.png (102.4 KB) - added by sabernhardt 2 months ago.
Editor with various lists in Twenty Eleven, in Arabic
51157.1.patch (7.0 KB) - added by amolv 2 months ago.
Patch refresh, RTL language support.
RTL language support.png (643.4 KB) - added by amolv 2 months ago.
Test after patch refresh.
2010-UL-TwentyTen.png (639.2 KB) - added by francina 2 months ago.
TwentyTen
2011-UL-TwentyEleven.png (784.4 KB) - added by francina 2 months ago.
Twenty Eleven Screenshot
2012-UL-TwentyTwelve.png (196.1 KB) - added by francina 2 months ago.
Twenty Twelve Screenshot
2013-UL-TwentyThirteen.png (688.3 KB) - added by francina 2 months ago.
Twenty Thirteen
2014-UL-TwentyFourteen.png (426.7 KB) - added by francina 2 months ago.
Twenty Fourteen
2015-UL-TwentyFifteen.png (621.8 KB) - added by francina 2 months ago.
Twenty Fifteen
2016-UL-TwentySixteen.png (519.4 KB) - added by francina 2 months ago.
Twenty Sixteen
2017-UL-TwentySeventeen.png (1.3 MB) - added by francina 2 months ago.
Twenty Seventeen
2019-UL-TwentyNineteen.png (486.7 KB) - added by francina 2 months ago.
Twenty Nineteen
2020-UL-TwentyTwenty.png (701.0 KB) - added by francina 2 months ago.
Twenty Twenty
example-posts-with-lists.xml (15.2 KB) - added by sabernhardt 8 weeks ago.
sample posts for both LTR and RTL list content
2020-nested-list-rtl.png (12.0 KB) - added by sabernhardt 8 weeks ago.
nested unordered list in a list block, Twenty Twenty, RTL
51157.2.patch (10.5 KB) - added by sabernhardt 6 weeks ago.
indenting nested lists and normalizing classic block lists
screenshots-after-51157.2.patch.zip (2.0 MB) - added by sabernhardt 6 weeks ago.
editor screenshots after 51157.2.patch

Change History (45)

#1 @SergeyBiryukov
3 months ago

  • Description modified (diff)
  • Milestone changed from Awaiting Review to 5.5.2

#2 @sabernhardt
3 months ago

  • Focuses css added

@amolv
2 months ago

Default List margin removed from theme.

#3 @amolv
2 months ago

  • Keywords has-patch added; needs-patch removed

twentyten, twentyeleven, twentythirteen, twentyfourteen, twentysixteen, twentyseventeen - in this themes, default margin of list is removed like #50029

Last edited 3 weeks ago by SergeyBiryukov (previous) (diff)

@audrasjb
2 months ago

After the patch (Twenty Eleven example)

#4 @audrasjb
2 months ago

  • Keywords has-screenshots added; needs-testing removed

I tested the proposed patch and it fixes the issue for each related bundled theme.
Marking this for commit.

#5 @audrasjb
2 months ago

  • Keywords commit added

#6 @sabernhardt
2 months ago

  • Focuses rtl added
  • Keywords needs-patch added; has-patch commit removed

The patch looks good with regular list blocks in LTR languages, but it still needs some adjustments for RTL support (even Twenty Fifteen).

@sabernhardt
2 months ago

Editor with various lists in Twenty Fifteen, in Arabic

@sabernhardt
2 months ago

Editor with various lists in Twenty Eleven, in Arabic

@amolv
2 months ago

Patch refresh, RTL language support.

@amolv
2 months ago

Test after patch refresh.

#7 @amolv
2 months ago

  • Keywords has-patch needs-testing added; needs-patch removed

CSS updated for RTL language support.

This ticket was mentioned in Slack in #core-css by kirstyburgoine. View the logs.


2 months ago

@francina
2 months ago

TwentyTen

@francina
2 months ago

Twenty Eleven Screenshot

@francina
2 months ago

Twenty Twelve Screenshot

@francina
2 months ago

Twenty Thirteen

@francina
2 months ago

Twenty Fourteen

@francina
2 months ago

Twenty Fifteen

@francina
2 months ago

Twenty Sixteen

@francina
2 months ago

Twenty Seventeen

@francina
2 months ago

Twenty Nineteen

@francina
2 months ago

Twenty Twenty

#9 @francina
2 months ago

Tested on a 16" Retina display, only LTR. The patch applies cleanly.
Twenty Fifteen and Twenty Seventeen seem to have very little margin on their left, so they are not consistent with the rest of the themes.

This ticket was mentioned in Slack in #core by monikarao. View the logs.


8 weeks ago

@sabernhardt
8 weeks ago

sample posts for both LTR and RTL list content

#11 @sabernhardt
8 weeks ago

Thanks for testing. Unfortunately, the patch is for editor styles, not for the front end. And while I would prefer having a margin to indent the lists, both Twenty Fifteen (at larger screen sizes) and Twenty Seventeen have had no left or right margin since each theme was released. Reconsidering that now would require a separate ticket and plenty of testing.

For further testing here, if anyone would like to import some starter/example posts, example-posts-with-lists.xml contains my lists for both LTR and RTL directions (I used Arabic, with Polylang plugin). Each post features simple ordered and unordered lists, plus nested lists (including mixed types). Then the same set of lists is repeated inside a full-width group block and a classic block. Other possible situations could be tested as well.

Last edited 8 weeks ago by sabernhardt (previous) (diff)

@sabernhardt
8 weeks ago

nested unordered list in a list block, Twenty Twenty, RTL

#12 @sabernhardt
8 weeks ago

  • Keywords needs-patch added; has-patch needs-testing removed

Twenty Twelve is fine with or without the patch.

All 7 of the themes in the patch now center simple lists in list blocks in both LTR and RTL languages. However, only Twenty Eleven shows nested lists in RTL list blocks properly, and Twenty Thirteen has trouble with nested lists in both LTR and RTL languages. Most themes have problems with nested lists and/or inconsistent list indentation in the classic block, too.

Also, the lack of any margin or padding on outer lists can result in cutting off the list markers at narrower widths. (This might be better to address within Gutenberg's styles, adding more than the 10px padding on each side.)

Twenty Nineteen and Twenty Twenty were not included in the patch. Edit: I'll open separate tickets for small fixes on those.

Last edited 6 weeks ago by sabernhardt (previous) (diff)

#13 @francina
8 weeks ago

  • Keywords needs-testing added

@sabernhardt my understanding from your last comment is:

  • For LTR and RTL non-nested lists, this is good to go and ready for commit
  • Only Twenty Eleven shows nested lists in RTL > Needs new ticket?
  • Twenty Thirteen has trouble with nested lists in both LTR and RTL languages > Needs new ticket?

Paging @ianbelanger for this, to assess if something can be done to get this into 5.6
Thanks!

#14 @sabernhardt
7 weeks ago

With patch 51157.1.patch (and the previous changeset [47934]), I can't tell if one list is inside another list, and that could be a bigger problem than having the block appear off to the side. So I would suggest not committing yet.

On the other hand:

  1. Twenty Fifteen already implemented this change (with LTR languages).
  2. As far as I know, the patch fixes the issue originally reported on this ticket.
  3. Any further work likely does not involve changing anything in the patch.
  4. Perhaps it's easier to create and test follow-up patches in separate tickets for each theme, instead of updating and testing all themes together, especially if Twenty Nineteen and Twenty Twenty are edited as well.

If you want to commit the main part now, I still would like to have at least most of the follow-up fixes in the same release. Whatever the decision is, I'll go with it.

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


6 weeks ago

#16 @audrasjb
6 weeks ago

  • Keywords dev-feedback has-patch added; needs-patch removed

As per today's bug scrub:
@sabernhardt so it looks like your last patch partially addresses the issue. My understanding is: if the current patch is committed, we’ll need some follow-up changes to address new issues, but I’m not sure those issues would be regression after that patch or pre-existing issues.

If the patch introduces some regressions, it’s of course better to directly address them in this ticket. If they are pre-existing issues, we can open another ticket to address them.

Can you clarify the current situation please?

#17 @sabernhardt
6 weeks ago

My patch was only a suggestion for Twenty Twenty, and I just deleted it to avoid confusion.

Amol's second patch 51157.1.patch centers lists in list blocks as intended—with both LTR and RTL languages—for 7 themes. A side effect is that nested lists are not always indented properly.

I'll see if I can create a patch to fix nested lists before Monday for the same 7 themes.

I can create new tickets for Twenty Nineteen and Twenty Twenty (attaching my previous patch there), for consideration separately. Those two should not delay the others.

@sabernhardt
6 weeks ago

indenting nested lists and normalizing classic block lists

#18 @sabernhardt
6 weeks ago

Building off of 51157.1 for Twenty Ten to Twenty Seventeen, 51157.2.patch:

  1. Indents nested lists in list and category blocks.
  2. Matches ordered and unordered indentation for the classic block's lists.

Twenty Seventeen still needed

  1. the left margin removed from RTL nested lists and
  2. ul:not(.wp-block-gallery) to override an earlier padding rule in editor-blocks.css

Twenty Thirteen required a different approach than the others, and now the ordered and unordered lists (in list blocks) have the same indentation as each other. (The full/half-width group block alignment is noted on #51440.)

Indentation values taken from styles.css and/or editor-styles.css:

  • 1.5em Twenty Ten
  • 2.5em Twenty Eleven
  • 40px Twenty Thirteen
  • 20px Twenty Fourteen
  • 23px Twenty Fifteen (from editor-styles.css)
  • 1.5em Twenty Sixteen (from style.css at min-width:44.375em)
  • 1.5em Twenty Seventeen

Screenshots:
https://www.flickr.com/gp/34028247@N03/3A7Z8X

@sabernhardt
6 weeks ago

editor screenshots after 51157.2.patch

#19 @sabernhardt
5 weeks ago

Follow-up tickets opened for Twenty Nineteen #51573 and Twenty Twenty #51574

#20 @sabernhardt
5 weeks ago

#51607 was marked as a duplicate.

#21 @whyisjake
4 weeks ago

  • Milestone changed from 5.5.2 to 5.5.3

Can we get a unified patch for all themes and all changes? Ideally, we get this merged in the next few hours, but I am going to move to the 5.5.3 milestone.

#22 @sabernhardt
4 weeks ago

  • Owner set to sabernhardt
  • Status changed from new to accepted

Twenty Nineteen and Twenty Twenty have separate tickets because their issue is not directly related to the list block centering, as is the case for the other 7 themes.

I'm confident 51157.2.patch fixes the original issue of centering because the previous patch did that, and I did not meddle with that part. If someone else is willing to test 51157.2.patch, please check that I didn't make a mistake or a poor choice—or cause new problems—with the extra changes for nested lists.

In addition to the List and Classic blocks in the sample example-posts-with-lists.xml posts, I tested it with Categories, Latest Posts, and Gallery blocks.

It may be too late for 5.5.2, but it's still good to try committing in time for 5.6.

#23 @JeffPaul
4 weeks ago

  • Milestone changed from 5.5.3 to 5.6

We're working on a short cycle 5.5.3 release in the very near future and have no further minor releases planned so I'm punting this to 5.6 in hopes that the patch can be validated for that major release.

This ticket was mentioned in Slack in #core-css by sabernhardt. View the logs.


3 weeks ago

#25 @SergeyBiryukov
3 weeks ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 49511:

Bundled Themes: Correct list block alignment in editor styles.

Additionally:

  • Indent nested lists in list and category blocks.
  • Match ordered and unordered indentation for the classic block's lists.

This applies to:

  • Twenty Ten
  • Twenty Eleven
  • Twenty Thirteen
  • Twenty Fourteen
  • Twenty Fifteen
  • Twenty Sixteen
  • Twenty Seventeen

Props sabernhardt, amolv, kjellr, audrasjb, francina.
Fixes #51157.

Note: See TracTickets for help on using tickets.