Make WordPress Core

Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#32176 closed defect (bug) (fixed)

default editor styles: no space around aligned image

Reported by: iseulde's profile iseulde Owned by: jmichaelward's profile 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 10 years ago.
32176.patch (431 bytes) - added by jmichaelward 10 years ago.
Added margin for alignleft, alignright, and aligncenter classes.
32176.2.patch (1.5 KB) - added by iseulde 10 years ago.
32176.3.patch (1.4 KB) - added by iseulde 10 years ago.

Download all attachments as: .zip

Change History (17)

#1 @iseulde
10 years ago

  • Keywords good-first-bug added

@jmichaelward
10 years ago

Added margin for alignleft, alignright, and aligncenter classes.

#2 @jmichaelward
10 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
10 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
10 years ago

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

#5 follow-up: @jmichaelward
10 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
10 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
10 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
10 years ago

Sounds great. Thanks, all, for the clarification!

#9 @obenland
10 years ago

@iseulde could you do a final review and commit?

#10 @obenland
10 years ago

@iseulde, should we do this in 4.4?

#11 @obenland
10 years ago

  • Milestone changed from 4.3 to Future Release

@iseulde
10 years ago

@iseulde
10 years ago

#12 @iseulde
10 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
10 years ago

  • Milestone changed from Future Release to 4.3

Sorry, lost track of this.

Note: See TracTickets for help on using tickets.