WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 2 years ago

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

Download all attachments as: .zip

Change History (39)

comment:1 @markjaquith2 years ago

  • Description modified (diff)

@rdall2 years ago

comment:2 @rdall2 years ago

  • Keywords has-patch added; needs-patch removed

comment:3 @obenland2 years 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: @nacin2 years 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: @RDall2 years 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 @obenland2 years 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 2 years ago by obenland (previous) (diff)

comment:7 in reply to: ↑ 4 @obenland2 years 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.

@obenland2 years ago

Adjusts caption text width in 2012 and 2013

@rdall2 years ago

Adjusts padding of ul ol to 15px from 40px

comment:8 @obenland2 years 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.

@rdall2 years ago

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

comment:9 follow-up: @lancewillett2 years 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: @lancewillett2 years 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 @RDall2 years 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 @lancewillett2 years 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: @lancewillett2 years 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 @RDall2 years ago

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

comment:15 @lancewillett2 years 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 @lancewillett2 years ago

Will need to apply to ol also.

comment:17 in reply to: ↑ 10 @markjaquith2 years 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 @bradparbs2 years ago

  • Cc brad@… added

@RDall2 years ago

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

comment:19 in reply to: ↑ 13 @RDall2 years 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: @lancewillett2 years 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: @RDall2 years 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 @lancewillett2 years 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.potukuchi2 years ago

  • Keywords reporter-feedback close added

comment:24 follow-up: @nacin2 years ago

  • Keywords reporter-feedback close removed

Any update here?

comment:25 in reply to: ↑ 24 @lancewillett2 years ago

  • Keywords needs-refresh added

Replying to nacin:

Any update here?

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

@rdall2 years ago

Fixes Editor style padding regarding the bullets

comment:26 @lancewillett2 years 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 @lancewillett2 years ago

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

@rdall2 years ago

Replaces sibling combinator with adjacent selector and improves code style

comment:28 follow-up: @rdall2 years 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 @lancewillett2 years 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 @lancewillett2 years 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: @rdall2 years 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 @SergeyBiryukov2 years 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.