WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#29986 closed defect (bug) (fixed)

Twenty Fifteen: Body margin in Visual Editor

Reported by: Jayjdk Owned by: azaozz
Milestone: 4.1 Priority: normal
Severity: normal Version: 4.1
Component: Bundled Theme Keywords: has-patch needs-testing
Focuses: Cc:

Description

The body margin in Twenty Fifteen is 83px 0 83px 83px;. While it matches the .entry-content margin on the front-end, it looks very weird on smaller screens. There's a huge white space on the left and top but none to the right. On bigger screens it looks okay (but in my opinion perhaps still too much)

I've attached a patch that uses 40px all around by default and uses media queries to use 83px when the screen is larger than 743px (660 + 83).

Attachments (5)

29986.patch (898 bytes) - added by Jayjdk 6 years ago.
29986.2.patch (746 bytes) - added by iseulde 6 years ago.
29986.3.diff (608 bytes) - added by iamtakashi 6 years ago.
Relative margin around body in the editor.
29986.4.patch (1.5 KB) - added by azaozz 6 years ago.
29986.4.diff (5.5 KB) - added by iamtakashi 6 years ago.
Change editor style with a medium body font size and 7px baseline grid.

Download all attachments as: .zip

Change History (23)

@Jayjdk
6 years ago

#1 @Jayjdk
6 years ago

Before:

http://i.imgur.com/xI0Tt2V.png

After:

http://i.imgur.com/coPAt34.png

#2 @Jayjdk
6 years ago

  • Keywords has-patch added

@iseulde
6 years ago

#3 @iseulde
6 years ago

  • Milestone changed from Awaiting Review to 4.1
  • Type changed from enhancement to defect (bug)

I agree, this looks weird, even when the window is a bit bigger (>743px) as it looks like the content is aligned right.
Maybe setting a percentage is better. Updated the patch.
Also note that list elements and block quotes may overflow the body, though with 10% it will never overflow html. On the front-end this is adjusted for smaller screen sizes, so I guess we need to do that for the editor too.

#4 @iseulde
6 years ago

azaozz fixed this partly in [29909]. We need a bigger margin though.

This ticket was mentioned in IRC in #wordpress-dev by azaozz. View the logs.


6 years ago

#6 @SergeyBiryukov
6 years ago

#30033 was marked as a duplicate.

#7 follow-up: @iamtakashi
6 years ago

I'm fine with 10%. Since [29911], max-width is broken though.

#8 in reply to: ↑ 7 @iamtakashi
6 years ago

Replying to iamtakashi:

I'm fine with 10%. Since [29911], max-width is broken though.

The max-width issue will be fixed when the default style is updated in #30038.
Ref: https://core.trac.wordpress.org/ticket/29799#comment:20

#9 @iandstewart
6 years ago

  • Keywords dev-feedback added

#10 @iandstewart
6 years ago

  • Keywords needs-testing added; dev-feedback removed

#11 follow-up: @iamtakashi
6 years ago

max-width issue is resolved in [29986]. Here is the update patch based on the suggestion from avryl.

#12 @azaozz
6 years ago

Been thinking/testing how to make this work well. We cannot use @media rules inside the editor as the iframe width has little to do with the main window width. It is possible to dynamically add/remove/replace classes on the iframe body and set the margins and font sizes with them. However that makes the editor look "strange" at window widths that are close to the breakpoints for collapsing the menu and moving the right widgets area under the editor.

As far as I see there are three options:

  • Decide on "average" margins and font size. Most compatible, will look slightly different than the front.
  • Dynamically add classes and change margins and font size depending on the main window width. Will look closer to the front-end in a wider window but starts to get problematic when the main window is narrower than 1300px.
  • Combination of the above, add a class only when the main window hits the 620px breakpoint.

@iamtakashi
6 years ago

Relative margin around body in the editor.

#13 in reply to: ↑ 11 ; follow-up: @azaozz
6 years ago

Replying to iamtakashi:

If we stay with the first option above (not adding body classes), thinking if may be better to go with the 17px font-size. Seems it looks good on all window sizes, from 2000px down to phones.

Also, the body margin seem to look better at 8% 10%.

Version 2, edited 6 years ago by azaozz (previous) (next) (diff)

#14 in reply to: ↑ 13 @iamtakashi
6 years ago

Replying to azaozz:

If we stay with the first option above (not adding body classes), thinking if may be better to go with the 17px font-size. Seems it looks good on all window sizes, from 2000px down to phones.

Also, the body margin seem to look better (more "even"?) at 8% 10%.

Yes, I personally prefer picking a size rather than trying to recreate pixel perfect reflection of the front which will add a lot of complexity to the editor style.

In this theme, there are three sizes for body text (15px, 17px, and 19px), so I also think the middle, 17px, is a good choice. I'll work on a new patch.

Last edited 6 years ago by iamtakashi (previous) (diff)

@azaozz
6 years ago

@iamtakashi
6 years ago

Change editor style with a medium body font size and 7px baseline grid.

#15 @iamtakashi
6 years ago

Oh, we were working on it at the same time :) I'll merge your changes and mine :)

#16 @azaozz
6 years ago

No need, already merged yours :)

#17 @azaozz
6 years ago

  • Owner set to azaozz
  • Resolution set to fixed
  • Status changed from new to closed

In 29987:

Twenty Fifteen: better editor-style.css: slightly smaller fonts, make the body margins relative, fix dependency on the mce-item-table class for tables. Props iamtakashi, fixes #29986

#18 @azaozz
6 years ago

Just FYI: the mce-item-table class for tables is toggled by the "View => Visual Aids" menu item and is "on" by default but can be turned off (not available in the WordPress default configuration as we don't show the TinyMCE menu).

Last edited 6 years ago by azaozz (previous) (diff)
Note: See TracTickets for help on using tickets.