WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#32176 closed defect (bug) (fixed)

default editor styles: no space around aligned image

Reported by: iseulde Owned by: jmichaelward
Milestone: 4.3 Priority: normal
Severity: normal Version:
Component: TinyMCE Keywords: good-first-bug has-patch needs-testing
Focuses: Cc:

Description

See screenshot. To reproduce you'll need no remove all editor styles, so install a theme without any, remove them manually from the DOM, or use remove_editor_styles().

Attachments (4)

Screen Shot 2015-04-28 at 18.04.24.png (144.8 KB) - added by iseulde 4 years ago.
32176.patch (431 bytes) - added by jmichaelward 4 years ago.
Added margin for alignleft, alignright, and aligncenter classes.
32176.2.patch (1.5 KB) - added by iseulde 4 years ago.
32176.3.patch (1.4 KB) - added by iseulde 4 years ago.

Download all attachments as: .zip

Change History (17)

#1 @iseulde
4 years ago

  • Keywords good-first-bug added

@jmichaelward
4 years ago

Added margin for alignleft, alignright, and aligncenter classes.

#2 @jmichaelward
4 years ago

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

I updated the wp-content.css stylesheet for TinyMCE, adding margin inspired by past Twenty* themes to the alignleft, aligncenter, and alignright classes.

#3 @McGuive7
4 years ago

Isn't this the theme's job, not WP's? I can see the case for adding a default margin here, however I wonder if it's not better left to the theme's editor stylesheet. It's my understanding that the default WP TinyMCE CSS is meant to be as minimal as possible to convey basic layout and sizing, and that the theme is then responsible for styling in-editor details such as margins, spacing, etc.

#4 @obenland
4 years ago

  • Owner set to jmichaelward
  • Status changed from new to assigned

#5 follow-up: @jmichaelward
4 years ago

Good afternoon, all.

@McGuive7 - This is my first time contributing to WordPress core, so I would like to hear @iseulde's reasoning for submitting this ticket. It was tagged as a "good first bug", so my patch merely addressed the concern without consideration for WordPress policy.

That said, I do think there is a place for minor presentational adjustments to be made in the core. WordPress has come a long way over the years in terms of improving the user experience on the backend, and I think reducing some of the work developers need to do to make the text editor a functional piece of their client's experience is beneficial to the project overall. Adding a sufficient amount of margin between images that are floated and the text they are next to seems like one of those minor details that improves the client presentation and reduces the need for developers to take time to address it. Including it only improves the experience, and it's unlikely devs will prefer to add styles to remove the margin.

Lastly, a question for @obenland: Since this is my first time contributing a patch, what does it mean that the ticket is assigned to me? Is there anything else I need to do to promote its inclusion in a future core release?

#6 in reply to: ↑ 5 @obenland
4 years ago

Replying to jmichaelward:

Since this is my first time contributing a patch, what does it mean that the ticket is assigned to me? Is there anything else I need to do to promote its inclusion in a future core release?

It's mostly about getting these tickets to show up under the Tickets with a contributor working on them. section of the Good First Bugs report.

#7 @iseulde
4 years ago

  • Milestone changed from Future Release to 4.3

We'll get this in. I just want to make sure the amount of pixels fits the rest of the theme.

Isn't this the theme's job, not WP's?

You need to be able to write properly without an editor stylesheet from the theme. An editor stylesheet is optional. The default experience should be good.

#8 @jmichaelward
4 years ago

Sounds great. Thanks, all, for the clarification!

#9 @obenland
4 years ago

@iseulde could you do a final review and commit?

#10 @obenland
4 years ago

@iseulde, should we do this in 4.4?

#11 @obenland
4 years ago

  • Milestone changed from 4.3 to Future Release

@iseulde
4 years ago

@iseulde
4 years ago

#12 @iseulde
4 years ago

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

In 33415:

Editor: add space around aligned images

Also increase the font size of captions.

Part props jmichaelward.
Fixes #32176.

#13 @iseulde
4 years ago

  • Milestone changed from Future Release to 4.3

Sorry, lost track of this.

Note: See TracTickets for help on using tickets.