Make WordPress Core

Opened 10 years ago

Closed 10 years ago

#25461 closed defect (bug) (fixed)

Edit published date of post - inputs with no labels

Reported by: grahamarmfield's profile grahamarmfield Owned by: johnbillion's profile johnbillion
Milestone: 4.0 Priority: lowest
Severity: minor Version: 3.6.1
Component: Editor Keywords: has-patch commit
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 10 years ago.
Screenshot of input fields to edit published date.
publish-date-visible-labels.jpg (18.1 KB) - added by grahamarmfield 10 years ago.
Rough screenshot showing visible labels
25461-date-input-labels.patch (3.0 KB) - added by buffler 10 years ago.
adds labels to post publish/schedule date
25461-date-input-labels.2.patch (3.0 KB) - added by buffler 10 years ago.
cleaner style for .inline-edit-date labels
25461-date-input-labels.3.patch (253.2 KB) - added by buffler 10 years ago.
25461-date-input-labels.4.patch (3.0 KB) - added by buffler 10 years ago.
25461-date-input-labels.5.patch (3.1 KB) - added by buffler 10 years ago.
uses <abbr> inside the labels so screenreaders get 'minute' & 'hour'
25461-date-input-labels.6.patch (3.1 KB) - added by buffler 10 years ago.
fixes .timestamp-wrap selector in wp-admin.css
publish-w-labels.png (19.5 KB) - added by buffler 10 years ago.
publish meta box w 25461-date-input-labels.6.patch applied
quick-edit-w-labels.png (15.4 KB) - added by buffler 10 years ago.
quick-edit w 25461-date-input-labels.6.patch applied
25461-date-input-labels.7.patch (3.1 KB) - added by buffler 10 years ago.
refreshed against trunk (currently at revision 26001)
25461-inline-edit-before-patch.jpg (22.2 KB) - added by seanchayes 10 years ago.
25461-inline-edit-post-patch-25461.7.jpg (24.1 KB) - added by seanchayes 10 years ago.
25461-inline-edit-post-patch-25461.8.jpg (22.5 KB) - added by seanchayes 10 years ago.
25461-date-input-labels.8.patch (3.5 KB) - added by seanchayes 10 years ago.
25461.patch (3.6 KB) - added by Frank Klein 10 years ago.
25461-alternate.patch (2.7 KB) - added by Frank Klein 10 years ago.

Download all attachments as: .zip

Change History (58)

@grahamarmfield
10 years ago

Screenshot of input fields to edit published date.

#1 follow-ups: @sharonaustin
10 years 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?

#2 in reply to: ↑ 1 ; follow-up: @helen
10 years 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. :)

#3 in reply to: ↑ 1 ; follow-up: @grahamarmfield
10 years 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.

#4 in reply to: ↑ 2 @sharonaustin
10 years 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.

#5 in reply to: ↑ 3 ; follow-up: @sharonaustin
10 years 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?

#6 in reply to: ↑ 5 ; follow-up: @grahamarmfield
10 years 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.

@grahamarmfield
10 years ago

Rough screenshot showing visible labels

#7 in reply to: ↑ 6 @sharonaustin
10 years 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!

#8 follow-up: @sharonaustin
10 years 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,

#9 in reply to: ↑ 8 ; follow-up: @buffler
10 years 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 10 years ago by buffler (previous) (diff)

@buffler
10 years ago

adds labels to post publish/schedule date

@buffler
10 years ago

cleaner style for .inline-edit-date labels

#10 in reply to: ↑ 9 @sharonaustin
10 years 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.

#11 @sharonaustin
10 years 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!

#12 follow-up: @buffler
10 years ago

  • Keywords has-patch added

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

#13 in reply to: ↑ 12 @helen
10 years 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.

#14 follow-up: @buffler
10 years 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.

#15 in reply to: ↑ 14 @LucP
10 years 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...

#16 follow-up: @buffler
10 years 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?

#17 follow-ups: @sharonaustin
10 years 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

#18 in reply to: ↑ 16 @grahamarmfield
10 years 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.

#19 in reply to: ↑ 17 ; follow-up: @grahamarmfield
10 years 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.

#20 in reply to: ↑ 19 @sharonaustin
10 years 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.

#21 @sharonaustin
10 years ago

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

#22 @sharonaustin
10 years 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

#23 in reply to: ↑ 17 @buffler
10 years 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?

@buffler
10 years ago

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

@buffler
10 years ago

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

#24 @buffler
10 years 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.

@buffler
10 years ago

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

@buffler
10 years ago

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

#25 @buffler
10 years ago

#25480 was marked as a duplicate.

#26 @SergeyBiryukov
10 years ago

  • Milestone changed from Awaiting Review to 3.8

@buffler
10 years ago

refreshed against trunk (currently at revision 26001)

#27 @SergeyBiryukov
10 years ago

  • Keywords ui-focus added

#28 follow-up: @seanchayes
10 years 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.

#29 in reply to: ↑ 28 @SergeyBiryukov
10 years 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?

#30 @seanchayes
10 years ago

Ah yes! Attached.

#31 @matt
10 years ago

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

#32 @grahamarmfield
10 years ago

Think patch needs refresh.

Version 0, edited 10 years ago by grahamarmfield (next)

#33 @grahamarmfield
10 years ago

  • Keywords needs-refresh added; has-patch removed

@Frank Klein
10 years ago

#34 @Frank Klein
10 years 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.

#35 follow-up: @nacin
10 years 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.

#36 @Frank Klein
10 years 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?

#37 in reply to: ↑ 35 @grahamarmfield
10 years 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.

#38 @grahamarmfield
10 years 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.

#39 @nacin
10 years ago

  • Component changed from Accessibility to Editor
  • Focuses accessibility added

#40 @SergeyBiryukov
10 years ago

  • Keywords commit added
  • Milestone changed from Future Release to 4.0

25461-alternate.patch looks like an improvement.

#41 @johnbillion
10 years ago

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

In 28730:

Add screen reader labels to the date inputs on the post editing screen. Fixes #25461. Props frank-klein.

Note: See TracTickets for help on using tickets.