WordPress.org

Make WordPress Core

Opened 7 months ago

Last modified 3 months ago

#25461 new defect (bug)

Edit published date of post - inputs with no labels

Reported by: grahamarmfield Owned by:
Milestone: Future Release Priority: lowest
Severity: minor Version: 3.6.1
Component: Editor Keywords: has-patch
Focuses: ui, accessibility Cc:

Description

In Edit Post screen in admin there is an option to edit the published date of a post - found in the Publish box.

When the link is clicked a select list and four text boxes appear - prompting the user to set the published date and time. None of these inputs has an associated label so that there meaning will not be evident to screen reader users.

Ideally, the prompts for each field would be visible too to help everyone understand what is expected, but if that is too controversial, then the labels can be hidden from sighted users - the visually-hidden class is useful here.

Attachments (17)

publish-date-boxes.jpg (20.1 KB) - added by grahamarmfield 7 months ago.
Screenshot of input fields to edit published date.
publish-date-visible-labels.jpg (18.1 KB) - added by grahamarmfield 7 months ago.
Rough screenshot showing visible labels
25461-date-input-labels.patch (3.0 KB) - added by buffler 6 months ago.
adds labels to post publish/schedule date
25461-date-input-labels.2.patch (3.0 KB) - added by buffler 6 months ago.
cleaner style for .inline-edit-date labels
25461-date-input-labels.3.patch (253.2 KB) - added by buffler 6 months ago.
25461-date-input-labels.4.patch (3.0 KB) - added by buffler 6 months ago.
25461-date-input-labels.5.patch (3.1 KB) - added by buffler 6 months ago.
uses <abbr> inside the labels so screenreaders get 'minute' & 'hour'
25461-date-input-labels.6.patch (3.1 KB) - added by buffler 6 months ago.
fixes .timestamp-wrap selector in wp-admin.css
publish-w-labels.png (19.5 KB) - added by buffler 6 months ago.
publish meta box w 25461-date-input-labels.6.patch applied
quick-edit-w-labels.png (15.4 KB) - added by buffler 6 months ago.
quick-edit w 25461-date-input-labels.6.patch applied
25461-date-input-labels.7.patch (3.1 KB) - added by buffler 5 months ago.
refreshed against trunk (currently at revision 26001)
25461-inline-edit-before-patch.jpg (22.2 KB) - added by seanchayes 5 months ago.
25461-inline-edit-post-patch-25461.7.jpg (24.1 KB) - added by seanchayes 5 months ago.
25461-inline-edit-post-patch-25461.8.jpg (22.5 KB) - added by seanchayes 5 months ago.
25461-date-input-labels.8.patch (3.5 KB) - added by seanchayes 5 months ago.
25461.patch (3.6 KB) - added by Frank Klein 5 months ago.
25461-alternate.patch (2.7 KB) - added by Frank Klein 5 months ago.

Download all attachments as: .zip

Change History (56)

grahamarmfield7 months ago

Screenshot of input fields to edit published date.

comment:1 follow-ups: sharonaustin7 months ago

Graham, I would like to take a stab at this one, as long as, it is something appropriate for new developers. Is it your opinion that this is something a new developer could tackle?

comment:2 in reply to: ↑ 1 ; follow-up: helen7 months ago

Replying to sharonaustin:

Graham, I would like to take a stab at this one, as long as, it is something appropriate for new developers. Is it your opinion that this is something a new developer could tackle?

Why not? Everybody has to start somewhere. :)

comment:3 in reply to: ↑ 1 ; follow-up: grahamarmfield7 months ago

Replying to sharonaustin:

Graham, I would like to take a stab at this one, as long as, it is something appropriate for new developers. Is it your opinion that this is something a new developer could tackle?

Yes Sharon, I think it would be a good one to start with - fairly straightforward.

Do you think the labels should be visible?

If you're going to hide them I realise I stated the visually-hidden class in my original ticket - the appropriate WordPress class is actually screen-reader-text I believe.

comment:4 in reply to: ↑ 2 sharonaustin7 months ago

Replying to helen:

Replying to sharonaustin:

Graham, I would like to take a stab at this one, as long as, it is something appropriate for new developers. Is it your opinion that this is something a new developer could tackle?

Why not? Everybody has to start somewhere. :)

Thank you Helen...I very much want to do this.

comment:5 in reply to: ↑ 3 ; follow-up: sharonaustin7 months ago

Replying to grahamarmfield:

Replying to sharonaustin:

Graham, I would like to take a stab at this one, as long as, it is something appropriate for new developers. Is it your opinion that this is something a new developer could tackle?

Yes Sharon, I think it would be a good one to start with - fairly straightforward.

Do you think the labels should be visible?

If you're going to hide them I realise I stated the visually-hidden class in my original ticket - the appropriate WordPress class is actually screen-reader-text I believe.

Graham, I'm not absolutely certain on the labels yet. I do tend towards the side that links should be visible because of switch users, of which we have many on campus. From what I understand, they rely on the visual cues provided. That said, I am much more ignorant than knowledgeable on the subject, and I would like to research you ticket on the visually-hidden class. I won't have a real chance to do that until this weekend....will that hold anything up?

comment:6 in reply to: ↑ 5 ; follow-up: grahamarmfield7 months ago

Replying to sharonaustin:

Graham, I'm not absolutely certain on the labels yet. I do tend towards the side that links should be visible because of switch users, of which we have many on campus. From what I understand, they rely on the visual cues provided. That said, I am much more ignorant than knowledgeable on the subject, and I would like to research you ticket on the visually-hidden class. I won't have a real chance to do that until this weekend....will that hold anything up?

My view is that they should be visible - not just for those with cognitive impairments, but to make life easier for everyone.

I've attached a rough screenshot with an idea.

grahamarmfield7 months ago

Rough screenshot showing visible labels

comment:7 in reply to: ↑ 6 sharonaustin7 months ago

Replying to grahamarmfield:

Replying to sharonaustin:

Graham, I'm not absolutely certain on the labels yet. I do tend towards the side that links should be visible because of switch users, of which we have many on campus. From what I understand, they rely on the visual cues provided. That said, I am much more ignorant than knowledgeable on the subject, and I would like to research you ticket on the visually-hidden class. I won't have a real chance to do that until this weekend....will that hold anything up?

My view is that they should be visible - not just for those with cognitive impairments, but to make life easier for everyone.

I've attached a rough screenshot with an idea.

Thank you, Graham, the screenshot helps greatly!

comment:8 follow-up: sharonaustin6 months ago

I worked on this over the weekend, and I've come to realize that this is more complicated than I first thought. I don't want to hold things up by holding on to a ticket I can't resolve in a timely manner, so I'm hoping a more advanced developer will take over here.

The problem is (and correct me if I'm wrong) that this is a JQUERY issue, more than the very simplest JQUERY that I can deal with.

As I understand it, the code in question involves the following file (for 3.6)

wp-admin/js/JQUERY/ui/jquery.ui.datepicker.min.js

Additionally, I also think it's important to involve code that can integrate with internationalization efforts, and I don't even begin to know how to do that in this case.

This ticket is far more complicated than I thought it would be when I first started it his weekend. I would rather another developer take this over, rather than have me sit on it and have nothing happen because of it. I am so sorry for any trouble I've caused here.

That said, I'll be watching this ticket with great interest, and with particular interest if there is any effort to manipulate code for viewstate in JQUERY. In the meantime, I'm going to look for another ticket for which I can make a contribution.

Best,

comment:9 in reply to: ↑ 8 ; follow-up: buffler6 months ago

Replying to sharonaustin:

The problem is (and correct me if I'm wrong) that this is a JQUERY issue...

I think the date inputs are actually generated by the function touch_time, located at line 627 of wp-admin/includes/template.php. This function is called in the 'Publish' meta box on a post/page edit page, when quick-editing in an 'All posts/pages' list, and in the 'Status' meta box on a comment edit page.

I have a patch going that works fine for me but might need some help, esp. with the translation side of things - but I'll attach it here so it can be improved upon (by yourself, Sharon, or anyone else who wants to improve it).

Last edited 6 months ago by buffler (previous) (diff)

buffler6 months ago

adds labels to post publish/schedule date

buffler6 months ago

cleaner style for .inline-edit-date labels

comment:10 in reply to: ↑ 9 sharonaustin6 months ago

Replying to buffler:

Replying to sharonaustin:

The problem is (and correct me if I'm wrong) that this is a JQUERY issue...

I think the date inputs are actually generated by the function touch_time, located at line 627 of wp-admin/includes/template.php. This function is called in the 'Publish' meta box on a post/page edit page, when quick-editing in an 'All posts/pages' list, and in the 'Status' meta box on a comment edit page.

I have a patch going that works fine for me but might need some help, esp. with the translation side of things - but I'll attach it here so it can be improved upon (by yourself, Sharon, or anyone else who wants to improve it).

This is hugely appreciated--thank you for not letting this sit. I will look again, someone made mention of the need for enabling translations, and I may work with him on that. Maybe I'll be able to make a (small) contribution after all.

comment:11 sharonaustin6 months ago

I just want to really thank buffer. FYI, I've incorporated the code into a shared test site for the accessibility group, (which is 3.7 beta not 3.6, for which this code was developed)with some real results...thank you so much. I will also incorporate into a 3.6 site the first chance I get.

http://www.red-hound.com/Test21334/wordpress/?page_id=2

The accessibility team meets in just a few minutes, at the #wordpress-ui channel, and I'll get their attention on what you've done here....we all really appreciate it! Thank you again!

comment:12 follow-up: buffler6 months ago

  • Keywords has-patch added

Realized I forgot to update wp-admin.min.css with the appropriate style changes. Refreshed patch attached.

comment:13 in reply to: ↑ 12 helen6 months ago

Replying to buffler:

Realized I forgot to update wp-admin.min.css with the appropriate style changes.

Actually, minified files should not be patched. Use the SCRIPT_DEBUG constant to use unminified versions instead.

comment:14 follow-up: buffler6 months ago

Gotcha. That makes sense; I just didn't think to look up whether there was protocol re: patching minified files. I'll attach a patch that doesn't include wp-admin.min.css, then. So, this 25461-date-input-labels.4 patch is almost the same as the .2 patch I uploaded earlier, but includes a change in the way a string was joined that I made prior to the .3 patch.

As far as I can tell the only thing that may need addressing is translation... I've done little to no work w/ translation stuff before. Wasn't sure if __( ) would cut it but that's what I've used here.

comment:15 in reply to: ↑ 14 LucP6 months ago

Replying to buffler:

Gotcha. That makes sense; I just didn't think to look up whether there was protocol re: patching minified files. I'll attach a patch that doesn't include wp-admin.min.css, then. So, this 25461-date-input-labels.4 patch is almost the same as the .2 patch I uploaded earlier, but includes a change in the way a string was joined that I made prior to the .3 patch.

As far as I can tell the only thing that may need addressing is translation... I've done little to no work w/ translation stuff before. Wasn't sure if __( ) would cut it but that's what I've used here.

your .4 patch works fine for me... The translation with __( ) is working as well, got the string back as a translatable object...

comment:16 follow-up: buffler6 months ago

related: #25480, also opened by grahamarmfield. I just saw it for the first time but it deals with the same issue in touch_time - so whenever this ticket is resolved it should resolve that one as well.

Maybe 25461-date-input-labels.4.patch is good to go?

comment:17 follow-ups: sharonaustin6 months ago

@buffler, this is awesome work. Would it be okay with you if I took the amazing work you've done on this already, and added a tiny bit more css styling? The issue is that I think it would make more sense to a screen-reader if instead of "HH" we could say, "hours", instead of "MM" we could say "minutes", and perhaps add the word "seconds" to the end.

I disabled the automatic updates for 3.7 on my test site, and applied the patch for touch-time 25461-date-input-labels.4.patch

You can see the results here:
http://www.red-hound.com/Test21334/wordpress/?p=57

comment:18 in reply to: ↑ 16 grahamarmfield6 months ago

Replying to buffler:

related: #25480, also opened by grahamarmfield. I just saw it for the first time but it deals with the same issue in touch_time - so whenever this ticket is resolved it should resolve that one as well.

Thanks buffler - I wondered if it might.

comment:19 in reply to: ↑ 17 ; follow-up: grahamarmfield6 months ago

Replying to sharonaustin:

@buffler, this is awesome work. Would it be okay with you if I took the amazing work you've done on this already, and added a tiny bit more css styling? The issue is that I think it would make more sense to a screen-reader if instead of "HH" we could say, "hours", instead of "MM" we could say "minutes", and perhaps add the word "seconds" to the end.

I disabled the automatic updates for 3.7 on my test site, and applied the patch for touch-time 25461-date-input-labels.4.patch

You can see the results here:
http://www.red-hound.com/Test21334/wordpress/?p=57

Indeed this is great work. Agree that the styling needs some attention to bring it all back into line.

Also think your suggestion of using hours and minutes is better - as it is likely to be able to be translated more easily, and of course it doesn't assume that someone understands the abbreviations.

I don't believe there is a seconds field in there, correct me if I'm wrong. 8-)

Am I able to access your server Sharon? I could test with Dragon and NVDA if you like.

comment:20 in reply to: ↑ 19 sharonaustin6 months ago

Replying to grahamarmfield:

Replying to sharonaustin:

@buffler, this is awesome work. Would it be okay with you if I took the amazing work you've done on this already, and added a tiny bit more css styling? The issue is that I think it would make more sense to a screen-reader if instead of "HH" we could say, "hours", instead of "MM" we could say "minutes", and perhaps add the word "seconds" to the end.

I disabled the automatic updates for 3.7 on my test site, and applied the patch for touch-time 25461-date-input-labels.4.patch

You can see the results here:
http://www.red-hound.com/Test21334/wordpress/?p=57

Indeed this is great work. Agree that the styling needs some attention to bring it all back into line.

Also think your suggestion of using hours and minutes is better - as it is likely to be able to be translated more easily, and of course it doesn't assume that someone understands the abbreviations.

I don't believe there is a seconds field in there, correct me if I'm wrong. 8-)

Am I able to access your server Sharon? I could test with Dragon and NVDA if you like.

Graham, absolutely...I didn't see all your responses while I was editing. You should have a login, but just in case, let me check your user name, and resend to you via email.

comment:21 sharonaustin6 months ago

@grahamarmfield I just resent you login information for the test site

comment:22 sharonaustin6 months ago

One other bit of information: As I understand the patch, it applies to 3.6.1. In addition to the test site above (red-hound.com/Test21334/wordpress/ which is version 3.7RC2), I had another test site which uses version 3.6.1 (red-hound.com/flamingo1/wordpress/). Maybe I was doing something wrong, but I was unable to find a line-for-line match for the patch in the file wp-admin/includes/template.php. My apologies if I've looked in the wrong location to apply the patch for 3.6.1.

See the screenshot from the other test site here:

http://red-hound.com/flamingo1/wordpress/?p=13

comment:23 in reply to: ↑ 17 buffler6 months ago

Replying to sharonaustin:

I think it would make more sense to a screen-reader if instead of "HH" we could say, "hours", instead of "MM" we could say "minutes"...

I had wondered about that before. I'm about to upload a patch that uses <abbr> inside the label. Seemed to me like a good compromise - screenreaders get the full translatable title, and the inline display of each label/input pair is maintained with an abbreviation (also translatable)... which leads me to:

You can see the results here:
http://www.red-hound.com/Test21334/wordpress/?p=57

The patch includes styles for the label for each field and the span wrapping each label/field pair - but because we shouldn't patch minified JS/CSS, to see the current styles in effect you'll have to define SCRIPT_DEBUG in your wp-config.php file - here are the codex instructions to do that, and I'll upload a screenshot of how this looks for me. (And thanks to helen for pointing that out earlier here - I'm learning here too...)

Also, re: your last note - my patches have been against whatever the current trunk version was at the time - I'm unsure of exactly the full version each time, but it's been 3.7-whatever. I updated my local copy today, so this new patch should be good against 3.7RC2.

Thoughts on the <abbr> thing? Am I right in thinking that's a good solution to the accessibility vs. clean display problem?

buffler6 months ago

uses <abbr> inside the labels so screenreaders get 'minute' & 'hour'

buffler6 months ago

fixes .timestamp-wrap selector in wp-admin.css

comment:24 buffler6 months ago

Geez - got ahead of myself...

.6.patch fixes a selector in wp-admin.css - so this should now be good. I'm also attaching screenshots to show what it looks like when you have the SCRIPT_DEBUG constant defined (so that you're viewing the changes to wp-admin.css and not the unchanged wp-admin.min.css.

buffler6 months ago

publish meta box w 25461-date-input-labels.6.patch applied

buffler6 months ago

quick-edit w 25461-date-input-labels.6.patch applied

comment:25 buffler6 months ago

#25480 was marked as a duplicate.

comment:26 SergeyBiryukov6 months ago

  • Milestone changed from Awaiting Review to 3.8

buffler5 months ago

refreshed against trunk (currently at revision 26001)

comment:27 SergeyBiryukov5 months ago

  • Keywords ui-focus added

comment:28 follow-up: seanchayes5 months ago

I had some time to test this and I thought I would share what I encountered - in Firefox 25 and Chrome 32 on Windows there were some visual issues with inline edit at the All Posts screen.

Before the patch the alignment of the inline-edit elements needed a little attention and the space in the input elements needed widening just a little.


Post 25461-date-input-labels.7.patch the alignment worsened


I made some CSS changes to mitigate the alignment issues I encountered and the result of these changes can be seen in the following image.


I have included the CSS rolled in with the original patch as 25461-date-input-labels.8.patch for additional review / testing on other systems.

And having just uploaded them I've seen vertical alignment issues with the "Private" checkbox before and after the patch.

comment:29 in reply to: ↑ 28 SergeyBiryukov5 months ago

Replying to seanchayes:

I have included the CSS rolled in with the original patch as 25461-date-input-labels.8.patch for additional review / testing on other systems.

Looks like the file is missing, could you attach it?

comment:30 seanchayes5 months ago

Ah yes! Attached.

comment:31 matt5 months ago

  • Priority changed from normal to lowest
  • Severity changed from normal to minor

comment:32 grahamarmfield5 months ago

Think patch needs refresh. It won't load against the latest version of trunk.

Last edited 5 months ago by grahamarmfield (previous) (diff)

comment:33 grahamarmfield5 months ago

  • Keywords needs-refresh added; has-patch removed

Frank Klein5 months ago

comment:34 Frank Klein5 months ago

  • Cc contact@… added
  • Keywords has-patch added; needs-refresh removed

Refreshed the patch, opting for a slightly different approach to avoid using <br /> tags. I'm not saying that using line breaks is a bad thing, it's just a personal preference of mine.

comment:35 follow-up: nacin5 months ago

  • Milestone changed from 3.8 to Future Release

I don't love the idea of showing these labels to sighted users. A better way forward might be to build a better date picker. But for now, let's reconsider that, and have an alternative patch that simply adds this for screen readers.

comment:36 Frank Klein5 months ago

Alternate patch that adds the labels only for screen readers.

While I agree that an improved date picker would be the better solution, I consider that this small fix will improve accessibility right now and until this better solution comes along, so why wait?

comment:37 in reply to: ↑ 35 grahamarmfield5 months ago

Replying to nacin:

I don't love the idea of showing these labels to sighted users. A better way forward might be to build a better date picker. But for now, let's reconsider that, and have an alternative patch that simply adds this for screen readers.

Whilst it's possible to have these fields for screen readers only, I would definitely favour these labels being visible here to help others - including voice recognition users, and those with cognitive impairments.

But I will test the new patch tomorrow (it's evening here) and report back.

comment:38 grahamarmfield4 months ago

Have tested the patch and I'm hearing the prompts for the date and time fields on both NVDA and JAWS. So the situation is improved for screen reader users.

comment:39 nacin3 months ago

  • Component changed from Accessibility to Editor
  • Focuses accessibility added
Note: See TracTickets for help on using tickets.