WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

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

Download all attachments as: .zip

Change History (39)

#1 @markjaquith
3 years ago

  • Description modified (diff)

@rdall
3 years ago

#2 @rdall
3 years ago

  • Keywords has-patch added; needs-patch removed

#3 @obenland
3 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.

#4 follow-up: @nacin
3 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.

#5 follow-up: @RDall
3 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.

#6 in reply to: ↑ 5 @obenland
3 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 3 years ago by obenland (previous) (diff)

#7 in reply to: ↑ 4 @obenland
3 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.

@obenland
3 years ago

Adjusts caption text width in 2012 and 2013

@rdall
3 years ago

Adjusts padding of ul ol to 15px from 40px

#8 @obenland
3 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.

@rdall
3 years ago

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

#9 follow-up: @lancewillett
3 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.

#10 follow-up: @lancewillett
3 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.

#11 in reply to: ↑ 9 @RDall
3 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.

#12 @lancewillett
3 years ago

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

#13 follow-up: @lancewillett
3 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.

#14 @RDall
3 years ago

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

#15 @lancewillett
3 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.)

#16 @lancewillett
3 years ago

Will need to apply to ol also.

#17 in reply to: ↑ 10 @markjaquith
3 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. :-)

#18 @bradparbs
3 years ago

  • Cc brad@… added

@RDall
3 years ago

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

#19 in reply to: ↑ 13 @RDall
3 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

#20 follow-up: @lancewillett
3 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.

#21 in reply to: ↑ 20 ; follow-up: @RDall
3 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.

#22 in reply to: ↑ 21 @lancewillett
3 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.

#23 @anand.potukuchi
3 years ago

  • Keywords reporter-feedback close added

#24 follow-up: @nacin
3 years ago

  • Keywords reporter-feedback close removed

Any update here?

#25 in reply to: ↑ 24 @lancewillett
3 years ago

  • Keywords needs-refresh added

Replying to nacin:

Any update here?

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

@rdall
3 years ago

Fixes Editor style padding regarding the bullets

#26 @lancewillett
3 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?)

#27 @lancewillett
3 years ago

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

@rdall
3 years ago

Replaces sibling combinator with adjacent selector and improves code style

#28 follow-up: @rdall
3 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… 

#29 in reply to: ↑ 28 @lancewillett
3 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/

#30 @lancewillett
3 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.

#31 follow-up: @rdall
3 years ago

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

#32 in reply to: ↑ 31 @SergeyBiryukov
3 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.