WordPress.org

Make WordPress Core

Opened 16 months ago

Last modified 4 weeks ago

#43587 assigned enhancement

UI adjustments to the Status box in the Edit Comment screen

Reported by: birgire Owned by: nfmohit
Milestone: 5.3 Priority: normal
Severity: normal Version:
Component: Comments Keywords: has-screenshots good-first-bug has-patch needs-design-feedback
Focuses: ui, administration Cc:

Description

Related to #43586

Also few suggestions for the Status box in the Edit Comment screen:

  • Add an underline on the box title (like in meta-box headers).
  • Change the box title from Status to Save.
  • Use the familiar layout from the post Publish meta-box.
  • Use the status layout from the post Publish meta-box. But maybe it's too many clicks?
  • Use the status format icon (don't know if it's only used for post status?)
  • Use the comments icon for the In response to row.

Location example: /wp-admin/comment.php?action=editcomment&c=123

Attachments (11)

before.jpg (17.4 KB) - added by birgire 16 months ago.
after.jpg (16.8 KB) - added by birgire 16 months ago.
edit-media-save-box.jpg (21.7 KB) - added by birgire 16 months ago.
43587.diff (6.6 KB) - added by nfmohit 16 months ago.
43587.2.diff (6.5 KB) - added by nfmohit 16 months ago.
Changes regarding coding standards
save-box-with-left-margin-on-form-inputs.jpg (21.5 KB) - added by birgire 16 months ago.
43587.3.diff (4.4 KB) - added by nfmohit 16 months ago.
Removes the comment status 'Edit' toggle and contains slight visual improvements and corrections
43587-4.diff (3.9 KB) - added by birgire 6 months ago.
from-43587-4.diff.jpg (19.4 KB) - added by birgire 6 months ago.
edit-mode-from-43587-4.diff.jpg (21.8 KB) - added by birgire 6 months ago.
before-and-after.jpg (26.5 KB) - added by birgire 4 months ago.

Download all attachments as: .zip

Change History (38)

@birgire
16 months ago

@birgire
16 months ago

#1 @melchoyce
16 months ago

  • Keywords needs-patch has-screenshots good-first-bug added

It seems ideal here to wholesale steal styles, or update the markup, to match the Editor status box.

#2 @afercia
16 months ago

I'd agree the visual proposed changes increase consistency and that's very welcome. However, I'd suggest to better consider a couple things:

  • title: I'm not sure "Save" would be appropriate, since a comment can't be "saved". The actual available actions are related to changing the status and the date.
  • I'd keep the 3 radio buttons, also for accessibility reasons. I'm not sure making the interaction more complicated, especially for keyboard and assistive technologies users, is a good option just for the sake of better visuals. I'd rather consider to keep it simple, as it is know.

#3 @birgire
16 months ago

@afercia thanks for your feedback.

  • Regarding the box title, I looked at the Edit Media screen and noticed it uses Save as the title. Attached the screenshot edit-media-save-box.jpg. But as we can in fact also modify other fields on the Edit Comment screen, like name, email, url and content, apart from the status and date, I thought Save would also make sense in this case. English is not my first language, so I guess there might be other better titles available ;-)
  • Regarding the status radio buttons, I agree. I now wonder if the post status settings on the Post Edit screen is sub-optimal regarding accessibility?

#4 @afercia
16 months ago

Ah right, I haven't considered the other fields! (English is not my native language too ;))

I now wonder if the post status settings on the Post Edit screen is sub-optimal regarding accessibility?

Well it is.. complicated :) We've tried to improve it but it's not possible to change it drastically, unless there's a complete re-design of the whole box. With the advent of Gutenberg, it will also be used less frequently.

#5 @birgire
16 months ago

Well it is.. complicated :) We've tried to improve it but it's not possible to change it drastically, > unless there's a complete re-design of the whole box. With the advent of Gutenberg, it will also be used less frequently.

hehe, probably better not to touch that ;-)

ps: Not related to this ticket, but I find it confusing that when we save a comment as "Approved" we are redirected to the front-end, otherwise if we save it as "Spam" or "Pending" we are redirected to the comments list table screen. So I feel a little sea-sick after playing with the Edit Comment screen ;-)

#6 @SergeyBiryukov
16 months ago

  • Milestone changed from Awaiting Review to 5.0

@nfmohit
16 months ago

#7 @nfmohit
16 months ago

  • Keywords has-patch added; needs-patch removed

The 435687.diff addresses all the enhancements mentioned in this ticket.

@nfmohit
16 months ago

Changes regarding coding standards

#8 @nfmohit
16 months ago

Please consider reviewing 43587.2.diff over 43587.diff as it contains slight changes regarding coding standards.

#9 @birgire
16 months ago

Thanks for the patch @nfmohit

The patch 43587.2.diff seems to work as expected to the original proposal.

@afercia suggested here (#2) that we should display the radio buttons by default and not use the extra "Edit" link to display it, for accessibility reasons.

I attached another screenshot in save-box-with-left-margin-on-form-inputs.jpg as a suggestion for that.

On that screenshot I also added an extra left margin on the holders of the form inputs. What do you think?

Do we need the bold status text, shown in the screenshot? It could help to see the previous status, if we've changed it (without saving). It's also in-line with e.g. the timestamp layout.

Another thing to consider are the icons. Currently the timestamp icon is added with CSS generated content. So it's natural that 43587.2.diff does the same for the status- and comments icons.

@afercia mentioned here:

"some screen readers already read out CSS generated content and more will do in the next future."

So should the icons be added via aria-hidden span tags here, instead of adding them via CSS generated content?

Version 4, edited 16 months ago by birgire (previous) (next) (diff)

@nfmohit
16 months ago

Removes the comment status 'Edit' toggle and contains slight visual improvements and corrections

#10 @nfmohit
16 months ago

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

Thank you so much for checking the patch out @birgire !

@afercia suggested here (#2) that we should display the radio buttons by default and not use the extra "Edit" link to display it, for accessibility reasons.

I'm sorry I completely misunderstood that part. I initially intended to make the comment status radio buttons mimic the post status selection as we can see in the posts and pages, but after @afercia's comment, I went with the radio buttons instead of the selection. Didn't realize we were referring to the "Edit" toggle link. My bad. I have removed that in this patch 43587.3.diff.

On that screenshot I also added an extra left margin on the holders of the form inputs. What do you think? (I opened the timestamp edit, just to better show the alignment there)

I really like the idea of pushing the form inputs a little to the right aligning them with their section headers, but I haven't added it in the patch yet as I think we require more opinions on this. I believe if we are going to implement this, it'll also apply to the similar sections like post status and visibility selection in posts and pages, as we are trying to keep the layout consistent between comments and post types. If everyone is okay with it, I'll update my patch with that applied. Opinions?

Do we need the bold status text, shown in the screenshot? It could help to see the previous status, if we've changed it (without saving). It's also in-line with e.g. the timestamp layout.

I really prefer keeping the bold text showing the current comment status, it is static now though and doesn't change when a different status is selected (until the comment is saved, of course).

So should the icons be added via aria-hidden span tags here, instead of adding them via CSS generated content?

I'd also prefer adding the icons via aria-hidden span tags, but I also need more opinions here as CSS generated content is being used consistently in other sections like post status, post visibility and timestamp in post types.

Personally, I think if we are going to add the left margins to the form holders and change to using aria-hidden span tags for the icons, we'd like to do that globally in posts and other sections as well. Opinions?

Thank you so much!

#11 @birgire
16 months ago

  • Resolution invalid deleted
  • Status changed from closed to reopened

@nfmohit I reopen the ticket, looks like you accidentally closed it as invalid ;-)

#12 @nfmohit
16 months ago

@birgire Whoops! I did? Yeah looks like I did. Sorry about that, it was a mistake. Thank you so much for reopening the ticket.

#13 @birgire
13 months ago

  • Owner set to nfmohit
  • Status changed from reopened to assigned

#14 @pento
9 months ago

  • Milestone changed from 5.0 to 5.1

#15 @pento
6 months ago

  • Keywords needs-design added

Patch needs design review.

#16 @pento
6 months ago

  • Milestone changed from 5.1 to 5.2

#17 follow-up: @melchoyce
6 months ago

Can someone add a screenshot of the latest patch?

@birgire
6 months ago

#18 in reply to: ↑ 17 ; follow-up: @birgire
6 months ago

Replying to melchoyce:

Can someone add a screenshot of the latest patch?

I refreshed the 43587.3.diff patch with 43587-4.diff.

Added the following screenshots:

#19 in reply to: ↑ 18 ; follow-up: @nfmohit
6 months ago

Replying to birgire:

Replying to melchoyce:

Can someone add a screenshot of the latest patch?

I refreshed the 43587.3.diff patch with 43587-4.diff.

Added the following screenshots:

Thank you for the refresh @birgire and apologies about the delay in response.

#20 in reply to: ↑ 19 @birgire
6 months ago

Replying to nfmohit:

Thank you for the refresh @birgire and apologies about the delay in response.

No problem and thanks for good points in comment:10.

I like the indentation on the comment's status choices and the comment's date fields.

To advance this ticket though, one might consider creating another ticket for the unified indentation?

#21 @afercia
4 months ago

Re: the "at" sign, it would be best to remove it and just use "at" in plain text. See https://core.trac.wordpress.org/ticket/44267#comment:10 :)

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


4 months ago

#23 follow-up: @melchoyce
4 months ago

The indentation is a little easier to scan, but it matches the Classic Editor, so let's move forward with the latest patch.

Maybe we could add indentation and swap out @ for "at" both here and in the Classic Editor in a new ticket.

#24 in reply to: ↑ 23 @birgire
4 months ago

Replying to melchoyce:

The indentation is a little easier to scan, but it matches the Classic Editor, so let's move forward with the latest patch.

Maybe we could add indentation and swap out @ for "at" both here and in the Classic Editor in a new ticket.

yes, that's sounds like a good plan to create separate tickets for the @ replacement and unified indentation.

I will look into that.

In the screenshot before-and-after.jpg we can see the before and after according to the latest patch in 43587-4.diff

Last edited 4 months ago by birgire (previous) (diff)

#25 @desrosj
4 months ago

  • Milestone changed from 5.2 to 5.3

The 5.2 beta deadline is just about here. Going to punt this one.

#26 @karmatosed
2 months ago

  • Keywords needs-design-feedback added; ui-feedback needs-design removed

This has design so removing that keyword and adding in design feedback one.

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


4 weeks ago

Note: See TracTickets for help on using tickets.