Make WordPress Core

Opened 12 years ago

Closed 11 years ago

Last modified 11 years ago

#21405 closed defect (bug) (fixed)

Twenty Twelve: Reply to comment not indented

Reported by: ronbme's profile ronbme Owned by: ronbme's profile RonBme
Milestone: 3.5 Priority: normal
Severity: normal Version:
Component: Bundled Theme Keywords:
Focuses: Cc:

Description

A reply to a comment is not indented. On same level as comment.

Using Twenty-Twelve theme. WP 3.5 alpha-21358

Attachments (3)

21405.diff (1.8 KB) - added by lancewillett 11 years ago.
Patch to fix both display issues: indentation and avatar spacing
wp-thrdcmnt-320px-before.png (23.5 KB) - added by paulwp 11 years ago.
threaded comments before
wp-thrdcmnt-320px-after-21405.png (26.1 KB) - added by paulwp 11 years ago.
threaded comments after

Download all attachments as: .zip

Change History (22)

#1 @SergeyBiryukov
12 years ago

  • Component changed from General to Bundled Theme
  • Milestone changed from Awaiting Review to 3.5
  • Summary changed from Reply to comment not indented to Twenty Twelve: Reply to comment not indented

#2 follow-up: @obenland
12 years ago

  • Cc konstantin@… added
  • Keywords close added

Twenty Twelve does not support threaded comments.

#3 in reply to: ↑ 2 ; follow-ups: @nacin
12 years ago

Replying to obenland:

Twenty Twelve does not support threaded comments.

That's lame.

#4 in reply to: ↑ 3 @ronbme
12 years ago

Replying to nacin:

Replying to obenland:

Twenty Twelve does not support threaded comments.

That's lame.

I agree. Instead of skipping comments and replies we are not interested in, we will now have to go through every comment.

I can see restricting the number of levels for replies.

#5 in reply to: ↑ 3 @lancewillett
12 years ago

  • Keywords needs-patch added; close removed

Replying to nacin:

Replying to obenland:

Twenty Twelve does not support threaded comments.

That's lame.

Hopefully you don't mean that literally (my leg does hurt, though). :)

This is probably just an oversight and can be easily addressed.

#6 @sennza
12 years ago

  • Cc bronson@… added

#7 @lancewillett
12 years ago

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

In [21393]:

Twenty Twelve: add CSS rule for threaded comments. Fixes #21405.

#8 follow-up: @paulwp
11 years ago

Hi all,

Threaded Comments Style

The CSS rule for threaded commments was put there too early, it should have been placed inside the Media queries section instead.

Having it where it currently is now will render threaded comments too narrow in small screen (320px iphone portrait viewport), this will limit you to be able to use only 2 level deep threaded comments.

In small screeen the children comments should not be styled with indent, it could just be same for all, or use different border'color, I don't know but the point is

White Space around Gravatar

Also the space (margins) around comment's gravatar is too much, eating up too much space in small device. Even there's only 1 level comment the proportion looks off too because the margin was design in normal screen situation. The CSS responsible for these margins should be adjusted differently in normal screen and small screen.

( i posted this at the forum and Ipstenu (Mika Epstein) adviced to put the comment here so )

#9 in reply to: ↑ 8 @lancewillett
11 years ago

  • Keywords needs-patch removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

Replying to paulwp:

Threaded Comments Style

The CSS rule for threaded commments was put there too early, it should have been placed inside the Media queries section instead.

Having it where it currently is now will render threaded comments too narrow in small screen (320px iphone portrait viewport), this will limit you to be able to use only 2 level deep threaded comments.

Can you upload a screenshot so we are looking at the same thing?

White Space around Gravatar

This should probably be a separate ticket since it's not related to the comment indentation display.

#10 @lancewillett
11 years ago

Probably at the very least we should reduce the indentation in mobile-first, then make it "normal" for larger viewports.

@lancewillett
11 years ago

Patch to fix both display issues: indentation and avatar spacing

@paulwp
11 years ago

threaded comments before

@paulwp
11 years ago

threaded comments after

#11 @paulwp
11 years ago

Hi lancewillett

Thanks for taking the time for this. The above are shots before and after the 21405.diff showing threaded comment indentation in small (320px) screen - it's showing 2nd, 3rd, 4th child - so we all can see that the 4th child is readable. ( for small screen this is more than enough )

#12 follow-up: @paulwp
11 years ago

Regarding the space around gravatar :

please notice 2 margins, the margin-left of the comment author's name and the margin-bottom of comment's header block, I think the proportion is a bit off, both in normal screen and small screen, but this depends on the design. (I mean it relates to overall look and feel, other margins, space around other elements of the theme in full, which depends on the designer.)

#13 in reply to: ↑ 12 @lancewillett
11 years ago

Replying to paulwp:

please notice 2 margins, the margin-left of the comment author's name and the margin-bottom of comment's header block, I think the proportion is a bit off, both in normal screen and small screen, but this depends on the design.

Right, I'll take a look at it.

#14 @lancewillett
11 years ago

  • Cc drewstrojny added

I've asked @drewstrojny for a quick opinion, also.

#15 in reply to: ↑ 3 ; follow-up: @drewstrojny
11 years ago

  • Cc drewstrojny removed

Two approaches I'd prefer (in order of preference):

  1. Enable comment thread indentation when we're above the breakpoint. If we're not above the breakpoint, just show all the comments with no indentation.
  2. Limit the comment threading to 2 (maybe 3) levels deep and reduce the indent a bit to accomodate smaller screens.

Also:

  • I don't like the idea of reducing the spacing between the avatar image and the name.
  • We should limit comment threading to 3 levels anyway by default to keep things sane.

paulwp mentioned iPhone resolution being 320px. iPhone has been at 640px + since the iPhone 4 (released almost 2 years ago). Resolutions will continue to go up, not back down. This further reinforces the first approach. Anyone on an older low resolution device doesn't need to see indentation on threaded comments.

#16 @drewstrojny
11 years ago

  • Cc dstrojny@… added

#17 in reply to: ↑ 15 @lancewillett
11 years ago

Replying to drewstrojny:

Two approaches I'd prefer (in order of preference):

Thanks for your input, Drew -- very helpful. Let's go with your first suggestion (no indentation until above the 600 px breakpoint) and we won't change anything with the margins around the avatar -- leaving it as-is.

#18 @lancewillett
11 years ago

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

In 22896:

Twenty Twelve: only use comment thread indentation when above the first breakpoint, to avoid super-narrow indented comments in small screens. Closes #21405.

#19 @paulwp
11 years ago

Hi Drew

paulwp mentioned iPhone resolution being 320px. iPhone has been at 640px + since the iPhone 4 (released almost 2 years ago).

A pixel is not a pixel.

The px in 320px is not physical pixel, it's the CSS pixel that browsers recognize as device width in media queries.

Resolutions will continue to go up, not back down. This further reinforces the first approach. Anyone on an older low resolution device doesn't need to see indentation on threaded comments.

There are 2 things going on here.

First, it's true that in small screen the indentation of threaded comments is not needed, it could just be flat or use other technique to mark the different ( like border's color ) but it depends on the design.

Second, the "low or high resolution" is still measured by the CSS pixel.

Note: See TracTickets for help on using tickets.