WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#21405 closed defect (bug) (fixed)

Twenty Twelve: Reply to comment not indented

Reported by: ronbme Owned by: RonBme
Milestone: 3.5 Priority: normal
Severity: normal Version:
Component: Bundled Theme Keywords:
Focuses: Cc:
PR Number:

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 7 years ago.
Patch to fix both display issues: indentation and avatar spacing
wp-thrdcmnt-320px-before.png (23.5 KB) - added by paulwp 7 years ago.
threaded comments before
wp-thrdcmnt-320px-after-21405.png (26.1 KB) - added by paulwp 7 years ago.
threaded comments after

Download all attachments as: .zip

Change History (22)

#1 @SergeyBiryukov
7 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
7 years ago

  • Cc konstantin@… added
  • Keywords close added

Twenty Twelve does not support threaded comments.

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

Replying to obenland:

Twenty Twelve does not support threaded comments.

That's lame.

#4 in reply to: ↑ 3 @ronbme
7 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
7 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
7 years ago

  • Cc bronson@… added

#7 @lancewillett
7 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
7 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
7 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
7 years ago

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

@lancewillett
7 years ago

Patch to fix both display issues: indentation and avatar spacing

@paulwp
7 years ago

threaded comments before

@paulwp
7 years ago

threaded comments after

#11 @paulwp
7 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
7 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
7 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
7 years ago

  • Cc drewstrojny added

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

#15 in reply to: ↑ 3 ; follow-up: @drewstrojny
7 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
7 years ago

  • Cc dstrojny@… added

#17 in reply to: ↑ 15 @lancewillett
7 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
7 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
7 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.