WordPress.org

Make WordPress Core

Opened 21 months ago

Closed 21 months ago

Last modified 21 months ago

#24797 closed defect (bug) (fixed)

Twenty Thirteen: Bullets conflicting with caption

Reported by: rdall Owned by: lancewillett
Milestone: 3.6 Priority: normal
Severity: normal Version: 3.6
Component: Bundled Theme Keywords: has-patch needs-refresh
Focuses: Cc:

Description (last modified by markjaquith)

When you float an image left then have an unordered list to its right, the bullets conflict with the caption.

Those bullets shouldn't hang out.
— MarkJaquith

Screenshot:

http://f.cl.ly/items/1u0D1e3N000n1D2b0L1U/Screen%20Shot%202013-07-18%20at%2010.02.51%20AM.png

Attachments (7)

24797.diff (326 bytes) - added by rdall 21 months ago.
24797.2.diff (898 bytes) - added by obenland 21 months ago.
Adjusts caption text width in 2012 and 2013
24797.3.diff (887 bytes) - added by rdall 21 months ago.
Adjusts padding of ul ol to 15px from 40px
24797.4.diff (1010 bytes) - added by rdall 21 months ago.
Fixed Menu error and inconsistencies in ul, ol in rtl also indents nested lists
24797.5.diff (1.1 KB) - added by RDall 21 months ago.
List style bullet is only positioned inside when floated against a image
24797.6.diff​ (1.0 KB) - added by rdall 21 months ago.
Fixes Editor style padding regarding the bullets
24797.7.diff (1.1 KB) - added by rdall 21 months ago.
Replaces sibling combinator with adjacent selector and improves code style

Download all attachments as: .zip

Change History (39)

comment:1 @markjaquith21 months ago

  • Description modified (diff)

@rdall21 months ago

comment:2 @rdall21 months ago

  • Keywords has-patch added; needs-patch removed

comment:3 @obenland21 months ago

That the bullets hang out is due to the fact the the floated image is wider than the list padding, I'm afraid.
The same effect can be seen in all previous Twenty themes:

Twenty Twelve: http://screencast.com/t/9Ncg2sSnTh2w
Twenty Eleven: http://screencast.com/t/xsoU1Zxmk
Twenty Ten: http://screencast.com/t/m0VbLH6KOv

We should probably increase the margin on floated, captioned images though, to give them more space.

comment:4 follow-up: @nacin21 months ago

The screenshot of Twenty Thirteen seems to indicate that the caption itself isn't limiting itself to the width of the image, and rather is overflowing. This is unlike 2010-2012.

comment:5 follow-up: @RDall21 months ago

So the

        list-style-position: inside;

of the bullets is bad form I did in my patch is bad form? Seems a simple solution.

comment:6 in reply to: ↑ 5 @obenland21 months ago

  • Milestone changed from Awaiting Review to 3.6

Replying to RDall:

So the list-style-position: inside; of the bullets is bad form I did in my patch is bad form? Seems a simple solution.

No, it actually makes a lot of sense :)
We just need to adjust padding a little bit.

Last edited 21 months ago by obenland (previous) (diff)

comment:7 in reply to: ↑ 4 @obenland21 months ago

Replying to nacin:

The screenshot of Twenty Thirteen seems to indicate that the caption itself isn't limiting itself to the width of the image, and rather is overflowing. This is unlike 2010-2012.

The width of the caption is the same as the caption container (image width + 10px), both in Twenty Twelve and Thirteen. It just happens to not be visible in this example.

@obenland21 months ago

Adjusts caption text width in 2012 and 2013

@rdall21 months ago

Adjusts padding of ul ol to 15px from 40px

comment:8 @obenland21 months ago

.3 goes in the right direction but has some inconsistencies:

Before: http://screencast.com/t/UcjiioYEQXe
With patch: http://screencast.com/t/KBNnIZcq3P3Q

We should also have the same padding for both LTR and RTL and the padding for menu shouldn't change.

@rdall21 months ago

Fixed Menu error and inconsistencies in ul, ol in rtl also indents nested lists

comment:9 follow-up: @lancewillett21 months ago

Is there any way to fix this edge-case (yes, I think it'll be uncommon) without changing the display of list items? I find it a lot harder to scan when the bullet is inside and not outside.

comment:10 follow-up: @lancewillett21 months ago

Those bullets shouldn't hang out.

That's pretty standard typography, bullets should hang left of in left-to-right text. Unless you meant "those bullets shouldn't overlap the caption" in which case, I agree.

For the hanging punctuation conventions, where bullets should be outside the margin of the list, see:

So it's both a matter of personal preference and readability -- and sticking with common conventions.

comment:11 in reply to: ↑ 9 @RDall21 months ago

Replying to lancewillett:

Is there any way to fix this edge-case (yes, I think it'll be uncommon) without changing the display of list items? I find it a lot harder to scan when the bullet is inside and not outside.

There is actually very little difference between to two visually.

http://f.cl.ly/items/3h2a0D3K3h1X1b2d3r2V/Screen%20Shot%202013-07-18%20at%202.52.16%20PM.png

Is the one the left is on the bullets on the inside… The one one the right is the bullets on the outside.

I didn't see the style bug until I put a list next to an image. But I think it is worth the change due to the small amount of code and the lack of visual change.

comment:12 @lancewillett21 months ago

@RDall try with lists on more than one line, you'll see how it affects the left edge after the first line.

comment:13 follow-up: @lancewillett21 months ago

One idea is to just target this exact case only.

.wp-caption + ul li {
    list-style-position: inside;
}

... for example. Or give a bit more spacing.

comment:14 @RDall21 months ago

I could give that ago this evening… I am just about to fly out the door.

comment:15 @lancewillett21 months ago

Going even further, could try only targeting left-aligned captions, right?

Giving the list a bit more left padding, for example.

.wp-caption.alignleft + ul {
	padding-left: 140px;
}

(Note, will need RTL equivalent.)

comment:16 @lancewillett21 months ago

Will need to apply to ol also.

comment:17 in reply to: ↑ 10 @markjaquith21 months ago

Replying to lancewillett:

Those bullets shouldn't hang out.

That's pretty standard typography, bullets should hang left of in left-to-right text. Unless you meant "those bullets shouldn't overlap the caption" in which case, I agree.

No, I meant the other thing. But I was ignorant of the typographical precedent. So hey, I learned something! But now I'm saying that the bullets shouldn't overlap the caption. :-)

comment:18 @bradparbs21 months ago

  • Cc brad@… added

@RDall21 months ago

List style bullet is only positioned inside when floated against a image

comment:19 in reply to: ↑ 13 @RDall21 months ago

I decided to go with the list style position because it was the smallest amount of code. And it works properly in both style.css and rtl.css

comment:20 follow-up: @lancewillett21 months ago

Let's stick with the padding technique only, and also apply to only left-aligned captions (and right-aligned) in RTL.

I don't think we need the "general sibling" selector here, just adjacent should do fine.

comment:21 in reply to: ↑ 20 ; follow-up: @RDall21 months ago

Replying to lancewillett:

Let's stick with the padding technique only, and also apply to only left-aligned captions (and right-aligned) in RTL.

  1. I honestly couldn't figure out how you were adding padding to the wp-caption… as much as I tried… I didn't want to set a defined size as the image would of course change size and reduce in the responsive nature of the theme.

I don't think we need the "general sibling" selector here, just adjacent should do fine.

  1. Ok… That being said I couldn't use the + selector in the other example because it would only apply to the element directly after the caption which in my case would have been a p tag and not the ul or ol So I had to use the ~.
  1. I could never get the exact same selector to apply in the editor-style.css for whatever reason… That last part has me scratching my head the most.

comment:22 in reply to: ↑ 21 @lancewillett21 months ago

Replying to RDall:

  1. I honestly couldn't figure out how you were adding padding to the wp-caption… as much as I tried… I didn't want to set a defined size as the image would of course change size and reduce in the responsive nature of the theme.

Oh, good point -- that means the list-style-position technique is more flexible (if worse-looking, heh).

I don't think we need the "general sibling" selector here, just adjacent should do fine.

  1. Ok… That being said I couldn't use the + selector in the other example because it would only apply to the element directly after the caption which in my case would have been a p tag and not the ul or ol So I had to use the ~.

Hmm. Also a good point. So if we use the general sibling all lists in a given post after a left-aligned caption will have the different bullet style. Oh well. That's an OK compromise.

  1. I could never get the exact same selector to apply in the editor-style.css for whatever reason… That last part has me scratching my head the most.

OK, please continue testing there, let's try to make it work.

So in summary: style left-aligned captions only and both ol and ul general siblings.

comment:23 @anand.potukuchi21 months ago

  • Keywords reporter-feedback close added

comment:24 follow-up: @nacin21 months ago

  • Keywords reporter-feedback close removed

Any update here?

comment:25 in reply to: ↑ 24 @lancewillett21 months ago

  • Keywords needs-refresh added

Replying to nacin:

Any update here?

@RDall can you make a new patch for us to test, please?

@rdall21 months ago

Fixes Editor style padding regarding the bullets

comment:26 @lancewillett21 months ago

@rdall Thanks for the new patch. Something's weird with the file, it's not recognized as a patch by Trac, though. I think there's a space after "diff " in the filename.

A few code notes:

  1. The patch removes a newline before "5.7 Post/Paging Navigation" which needs to be there.
  2. There should be an extra newline before " * 3.0 Basic Structure" comment, also.
  3. Please use tabs, not spaces for indentation.
  4. Why do you have both general sibling and adjacent selectors? I see one block around line 748 and the other at the very bottom of the file. (Remove the bottom one, right?)

comment:27 @lancewillett21 months ago

@rdall Nice technique on the mceTemp selector. I'd suggest changing that to general sibling selector, also: ~.

@rdall21 months ago

Replaces sibling combinator with adjacent selector and improves code style

comment:28 follow-up: @rdall21 months ago

@lancewillett

Uploaded new patch with the adjacent selector. Can I ask why you require two lines before every category heading in the CSS file. Is it just style core has developed or something else that requires those two lines… 

comment:29 in reply to: ↑ 28 @lancewillett21 months ago

Replying to rdall:

@lancewillett

Uploaded new patch with the adjacent selector.

Hmm, looks like cruft crept back in with "Padding for caption against crowding bullets" block. I can ignore it. :)

Can I ask why you require two lines before every category heading in the CSS file. Is it just style core has developed or something else that requires those two lines… 

Yes, it's a coding standard. See http://make.wordpress.org/core/handbook/coding-standards/css/

comment:30 @lancewillett21 months ago

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

In 24785:

Twenty Thirteen: better display for lists next to left-aligned captions. Props rdall for initial patches, closes #24797.

comment:31 follow-up: @rdall21 months ago

What does "cruft crept back in" actually mean? And does this mean I actually have patch twenty thirteen?

comment:32 in reply to: ↑ 31 @SergeyBiryukov21 months ago

Replying to rdall:

What does "cruft crept back in" actually mean?

Just that the last change in 24797.7.diff is a part of 24797.5.diff that should have been removed per comment:22.

Note: See TracTickets for help on using tickets.