Make WordPress Core

Opened 8 years ago

Last modified 6 years ago

#36424 new enhancement

graphically visualize if comments are closed (in wp-admin/edit.php)

Reported by: pixelverbieger's profile pixelverbieger Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Comments Keywords: has-patch has-screenshots has-ux-feedback
Focuses: ui, administration Cc:

Description

it would be nice if the pages or posts list would not only show the number of comments per page but if it also could show a symbol if comments for this particlar page or post are closed.

proposal:

  • the lock symbol

https://developer.wordpress.org/resource/dashicons/#lock

  • or

https://developer.wordpress.org/resource/dashicons/#edit
combined with a kind of "forbidden" sign/strike, similar to
https://developer.wordpress.org/resource/dashicons/#hidden

Attachments (10)

comments-closed.jpg (29.6 KB) - added by pixelverbieger 8 years ago.
where the new symbol should show up
class-wp-list-table.php.patch (1.1 KB) - added by Presskopp 8 years ago.
36424.2.diff (583 bytes) - added by Presskopp 8 years ago.
36424.jpg (31.7 KB) - added by Presskopp 8 years ago.
36424lotsoflocks.jpg (31.9 KB) - added by Presskopp 8 years ago.
showing open lock
36424.patch (571 bytes) - added by rachelbaker 8 years ago.
Moved aria-hidden attribute into span with icon classes
looks-ok.png (6.5 KB) - added by Presskopp 8 years ago.
looks ok
too-far-to-the-right.png (9.6 KB) - added by Presskopp 8 years ago.
too far to the right
comments-closed-1.png (16.9 KB) - added by LittleBigThing 8 years ago.
Different visual proposed for Comments closed
comments-closed-2.png (25.0 KB) - added by LittleBigThing 8 years ago.
Results of the above for different situations

Download all attachments as: .zip

Change History (52)

@pixelverbieger
8 years ago

where the new symbol should show up

#1 follow-up: @ocean90
8 years ago

  • Component changed from Administration to Comments
  • Focuses ui administration added

Hello @pixelverbieger, welcome to Trac!

I like the idea. Would you like to work on that in form of a patch?

#2 in reply to: ↑ 1 @pixelverbieger
8 years ago

Replying to ocean90:

Hello @pixelverbieger, welcome to Trac!

I like the idea. Would you like to work on that in form of a patch?

Sorry Dominik, this exceeds my knowledge. I don't know neither OOP nor GIT …

#3 @Presskopp
8 years ago

I have a pre-patch, or patch-idea:

Apply the following change in

wp-admin\includes\class-wp-list-table.php:658 (wordpress-4.5-RC1)


                // Comments are closed, show a lock then - Translation needed
                if (!comments_open( $post_id )) {
                        printf( '<span aria-hidden="true"><span class="dashicons dashicons-lock"></span></span><span class="screen-reader-text">%s</span>',
                                __( 'Comments closed' )
                        );
                // No comments at all.
                } elseif ( ! $approved_comments && ! $pending_comments ) {
                        printf( '<span aria-hidden="true">—</span><span class="screen-reader-text">%s</span>',
                                __( 'No comments' )
                        );

Now you may feed the fish with it :-)

Last edited 8 years ago by Presskopp (previous) (diff)

#4 follow-up: @Presskopp
8 years ago

I had a night to think about it. What if there is at least 1 comment AND comments are closed. Do we show the 'comments closed' sign or do we show the bubble with the number? Or both??

The other thing is the line for screen reader - "Comments are closed." seems to be the standard throwout in this case inside WP and not just "Comments closed".
I'm not sure about if a new line for translation is needed here, or if it gets done when we use "Comments are closed" - and what about the dot at the end of the string? "Comments are closed." Is it needed or not?

This is showing the lock only when there also are no comments:

                // Comments are closed, show a lock then - Translation needed
                if ( ! comments_open( $post_id ) && ( ! wp_count_comments( $post_id ))) {
                        printf( '<span aria-hidden="true"><span class="dashicons dashicons-lock"></span></span><span class="screen-reader-text">%s</span>',
                                __( 'Comments are closed' )
                        );

#5 in reply to: ↑ 4 @pixelverbieger
8 years ago

+1 for showing both number and lock

#6 @Presskopp
8 years ago

This is a little more complicated than I thought. My code above is rubbish :)

I think we need to use st. like

if ( ! comments_open( $post_id ) && ( empty(wp_count_comments( $post_id )->total_comments))) {

What about comments in trash? Do they count?

We could divide this into many cases, but it could also lead to an overkill.

I'll wait now for some reaction of someone who understands more than I.

Last edited 8 years ago by Presskopp (previous) (diff)

#7 @pixelverbieger
8 years ago

I think comments in trash should not count.

#8 @Presskopp
8 years ago

Latest brainstorming, at least it's working..

I thought if we show a locked lock we can also switch to an open lock if comments are allowed. And why not change the boring mdash for a sexy X ? ;-)
Another interesting dashicon candidate would be 'dashicons-welcome-comments' (it is already used in the dashboard welcome panel):
https://developer.wordpress.org/resource/dashicons/#welcome-comments

Still not sure about how to deal with translations here.

And I ask myself where core ends and plugins territory begins.

However, here something to play around:

                // If comments are closed, show a lock then
                if ( ! comments_open( $post_id ) ) {
                        printf( '<span aria-hidden="true"><span class="dashicons dashicons-lock"></span></span><span class="screen-reader-text">%s</span>',
                                __( 'Comments are closed' )
                        );
                }
                // Comments are allowed, show open lock
                else {
                        printf( '<span aria-hidden="true"><span class="dashicons dashicons-unlock"></span></span><span class="screen-reader-text">%s</span>',
                                __( 'Comments are allowed' )
                        );
                }
                // No comments at all, show X icon instead of mdash
                if ( ! $approved_comments && ! $pending_comments ) {
                        printf( '<span aria-hidden="true"><span class="dashicons dashicons-no-alt"></span></span><span class="screen-reader-text">%s</span>',
                                __( 'No comments' )
                        );

Last edited 8 years ago by Presskopp (previous) (diff)

#9 @pixelverbieger
8 years ago

but an X is ambiguous. it can stand for a yes and a no. this depends on the context! in a feature list or table the X oftentimes stands for is set or exists or included

Last edited 8 years ago by pixelverbieger (previous) (diff)

#10 @Presskopp
8 years ago

Yes, this X-thing was more a "feasibility study" :)
So you like the rest of it? Did you try?

#11 @pixelverbieger
8 years ago

I think permanently showing a lock icon is not that good. That's too much.
Showing a locked lock only if it applies, makes it far more outstanding than showing a lock (being closed or open) in every line.

And the icon dashicons-no-alt looks like a button indicating you can close something ;)

What about this:

if comments == 0 and comments open -> show mdash and nothing else (no lock)

if comments != 0 and comments open -> show number of comments and nothing else (no lock)

if comments == 0 and comments closed -> show mdash and locked lock, maybe in brackets

if comments != 0 and comments closed -> show number of comments and locked lock, maybe in brackets

The number of comments should come first, the lock symbol second.

Last edited 8 years ago by pixelverbieger (previous) (diff)

#12 @Presskopp
8 years ago

I like the idea to show the lock to the right of the bubble, I don't like brackets.

That would just be adding this at

Line 689:

                // If comments are closed, show a lock then
                if ( ! comments_open( $post_id ) ) {
                        printf( '<span aria-hidden="true"><span class="dashicons dashicons-lock"></span></span><span class="screen-reader-text">%s</span>',
                                __( 'Comments are closed' )
                        );
                }

I tried to join git to provide a correct patch, but until now I'm not fully understanding the process :(

#13 @pixelverbieger
8 years ago

Your code does what I initially meant when writing down my idea. Great!

The next step is to adjust the alignment of bubble and lock :)
Which is not trivial, because the dash and the bubble have very different heights, so it is not done with a simple
.dashicons-lock { margin-top:5px; }

(The bubble gets an margin-top of 5px applied, coming from line 40-46 in
wp-admin/css/list-tables.css
I think.)

#14 @kebbet
8 years ago

Just a thought: What about adding the ability to close and open comments på clicking the lock icon? Is it needed, or wanted?

#15 @Presskopp
8 years ago

Still learning, please have some patience with me, thank you. Trying to attach a patch here.

Don't know what's happening at line 28/29 - it wasn't me

I will wait for any further discussion (CSS, translation, events) happening here or not.

Last edited 8 years ago by Presskopp (previous) (diff)

This ticket was mentioned in Slack in #core-comments by presskopp. View the logs.


8 years ago

#17 @rachelbaker
8 years ago

  • Keywords has-patch needs-screenshots needs-refresh added
  • Milestone changed from Awaiting Review to Future Release

As discussed with @Presskopp during our bug scrub in #core-comments, the patch needs a refresh so lines 695-700 are no longer commented out. Refreshed screenshots would also allow this ticket to go on to the next step and receive UX feedback.

@Presskopp
8 years ago

@Presskopp
8 years ago

#18 @Presskopp
8 years ago

Patch cleaned (to the bone), Screenshot added

#19 @rachelbaker
8 years ago

  • Focuses accessibility added

@afercia Would we even need to include aria-hidden="true" here? Any other accessibility concerns?

#20 @rachelbaker
8 years ago

  • Keywords has-screenshots ux-feedback added; needs-screenshots needs-refresh removed

@Presskopp
8 years ago

showing open lock

#21 @Presskopp
8 years ago

last screenshot to show the version with also an open lock, which me and @rachelbaker wouldn't prefer (right, Rachel?), so no patch for that attached.

#22 @afercia
8 years ago

@rachelbaker yep it's always better to hide CSS generated content with aria-hidden since some screen readers (e.g. VoiceOver) tend to consider it like a "text element". Maybe aria-hidden could be set on the dashicon span, saving one span? No other accessibility concerns, as long as there's some screen-reader-text that assistive technologies can read out.
Not so convinced about the visual part, but will leave this to the design team :)

#23 @Presskopp
8 years ago

I scanned through the WP-Files, looking for span following span for candidates to possibly be combined and I really think it's not worth the effort.

@rachelbaker
8 years ago

Moved aria-hidden attribute into span with icon classes

#24 @rachelbaker
8 years ago

In 36424.patch I merged two of the span elements and generated the patch from the root directory.

Updated screenshots:

Posts list table without the patch applied:
https://cldup.com/9uI9TD0nXp.png

Posts list table with the patch applied:
https://cldup.com/ujRnvLIeM--3000x3000.png

I like the concept of showing a lock for posts with locked comments, but the display looks cluttered to me. Maybe we can get some thoughts from @melchoyce or @michaelarestad here.

#25 @Presskopp
8 years ago

I had to edit my post heavily, sorry. All I used for screens is the latest patch and this tiny CSS:

.dashicons-lock {
    margin-top: 5px;
}

but I don't see how I could set a minus margin-left for .dashicons-lock following .post-com-count-pending

Last edited 8 years ago by Presskopp (previous) (diff)

@Presskopp
8 years ago

looks ok

@Presskopp
8 years ago

too far to the right

#26 @pixelverbieger
8 years ago

Both your latest PNGs look good to me.
Very nice!

@LittleBigThing
8 years ago

Different visual proposed for Comments closed

#27 @LittleBigThing
8 years ago

Nice idea (this ticket).

What about something like above? (Sorry, image was added before I knew it, first time :-)).

Last edited 8 years ago by LittleBigThing (previous) (diff)

#28 @pixelverbieger
8 years ago

nice, too!

#29 @LittleBigThing
8 years ago

Some additional stuff if we would go down this way.

Assuming that the latest patch is slightly modified:

  • adding an additional class post-com-closed
  • adding the code before row 658. This means that 'Comments are closed' comes first in HTML markup (if this isn't ideal for accessibility, it would be easier to put the lock in the bottom right corner CSS-wise)

the following CSS would work quiet well:

.post-com-closed {
    position: relative;
    left: 3px;
    width: 17px;
    height: 17px;
    border: 2px solid #fff;
    border-radius: 11px;
    background: #ccc;
    color: #fff;
  }
.post-com-closed:before {
    font-size: 12px;
    position: relative;
    top: -6px;
}

The color of the lock could/might be darker (like 72777c) if necessary with regard to accessibility.

Another point, the text 'Comments are closed' should probably be displayed on mobile instead of the lock (as for the number of (unapproved) comments). This is not yet the case in this proposal, but some additional CSS would do it.

@LittleBigThing
8 years ago

Results of the above for different situations

This ticket was mentioned in Slack in #design by karmatosed. View the logs.


8 years ago

#31 @karmatosed
8 years ago

This question came up in the design chat and is really important to ask. Why are we adding this indication here? What importance does it have to the user at this point? What does it add to their use knowing this state here through the icon indication?

Personally, I'm not convinced at this stage it does add anything or is important. I'm not sure what behaviour or process is improved or helped knowing it's locked.

#32 @Presskopp
8 years ago

If we expect it as the usual behaviour of having comments enabled (do we?), this would be a reminder for pages which have comments closed, so they can be reopened or whatever. And it also would show the state of other posts indirectly (no lock = comments allowed, everything is fine here). Or if one would like all comments closed sitewide, he could see they definitely are.
We would need to know for what reasons people are closing/(re)opening their comments to get a feeling if this was a helpful enhancement or just children's playing. :)

#33 @LittleBigThing
8 years ago

Just read the conversation on Slack #design. I agree that it becomes clutter and I am also not sure whether it is useful information on the post list screen.

What I would find useful is to be able to see at a glance that comments are closed on (certain) posts/pages if I have closed commenting by the general option. I can find out using 'Quick edit', but that's post by post... 'Bulk edit' is not useful for this.


Maybe an interesting point: comments allowed/closed is the only thing that is not visible at a glance in the list table by default (besides the slug). All things editable using 'Quick edit' are there, except for this (and the slug). :-)


Further, the defaults are not shown explicitly in the list table.
For example, since posts are not sticky by default, if a post is sticky it is shown in the table. If a post is password protected, it is shown in the list table. So on...
So if we follow this thinking (which is logical), this would mean that indicating whether comments are allowed on a post/page is useful if it is based on the general option (Settings/Discussion). For example, if comments are allowed in general, then it may be interesting to see if comments are closed on certain posts/pages (using a lock like above). But if comments are closed in general, then it could be indicated if comments are allowed for certain posts/pages, using an open lock for example.
This of course a bit trickier to implement, but might be more useful and would make more sense. This would also avoid the possible clutter for most sites for posts, since the general option equals mostly to the option for an individual post.
Well, I hope this reasoning makes sense anyway. :-)

It is also interesting to think about how such a change would affect the user after updating to a new version of WordPress supporting this feature... it could be confusing.

#34 @pixelverbieger
8 years ago

Having read all your posts and the discussion on slack, I can say: @LittleBigThing got it and the better title for the ticket probably would be

"graphically visualize if comments are different from the general setting in the blog"

@karmatosed

In my opinion, the benefit for the user is:

If someone is wanting comments on his site and unintentionally closed them on a particular post/page, it would be good for him to see the mistake and thus be able to correct it.

On the other hand, if someone does not want pages/posts to be commented (and makes this as a general seeting) but for some reason one page or post is published with comments open, it also would be good to see this mistake to be able to correct it.

Not always is the one big default switch suitable for all posts and pages in a website and then it is useful to see at a glance how the setting are in detail.

And if someone is not at all interested in comments and related stuff like status and number of comments, he can switch off the column.

#35 @Presskopp
8 years ago

Hmm,

then instead of a lock we could show an exclamation mark (!) (or asterisk or similar) when a posts comment setting differs from the global setting - because we expect the user to know if he wants comments in general or not (and did that setting), and all he would need to benefit from is a 'Hey, look, something is different here'

:)

Last edited 8 years ago by Presskopp (previous) (diff)

#36 @pixelverbieger
8 years ago

addition:
@karmatosed for me a lock does not represent "passwort protected" -> this would be a key, no?

helen said "We already use it for post edit locking" – but that's in a different column, isn't it? So there should be no problem; lock near edit link: editing is locked; lock in comments column: comments locked.

:)

#37 @Presskopp
8 years ago

To the question what icon could be used if not a lock, please see above:
https://core.trac.wordpress.org/ticket/36424#comment:8

#38 @Presskopp
8 years ago

To bring a little more confusion in ;-) , we didn't even discuss possibilities like this yet:

if ( ! pings_open( $post_id ) ) { 
   printf( '<span aria-hidden="true" class="dashicons dashicons-hidden"></span><span class="screen-reader-text">%s</span>', __( 'Pingbacks/Trackbacks are not allowed' ) ); 
}       

No, I don't want it like that - just a thought I had to share.

This ticket was mentioned in Slack in #design by karmatosed. View the logs.


7 years ago

#40 @karmatosed
7 years ago

  • Keywords has-ux-feedback added; ux-feedback removed

We discussed this in today's design chat again as we go through triage. The consensus was that this still isn't something we advise adding. I personally stand by my comment from a while ago, it just doesn't seem the right thing for user to have. I think it adds noise over need.

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


6 years ago

#42 @afercia
6 years ago

  • Focuses accessibility removed

Accessibility feedback has been given, removing the focus to keep the a11y reports clean.

Note: See TracTickets for help on using tickets.