Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Spin Button Date Picker Example: Fix incrementing and decrementing past limits #1154

Merged
merged 2 commits into from
Sep 28, 2019

Conversation

backwardok
Copy link
Contributor

The date picker spin button example would show NaN on all the dates if you tried to loop around to the first date. Also, despite there being a min and max year set, the year could be cycled through potentially infinitely beyond the min or max year.

This was due to the way the min and max values were being queried. The values are set on the DOM nodes for the node with the spinbutton role. However, the node that was being queried for those values was the parent node.

This change fixes the node used so that the initial values are set appropriately, and the spinners work as expected.

Before, once you tried to wrap around beyond the last day of the month, all the values would then be set to NaN, and there'd be no way to get it to go back to having real numbers without refreshing the page. Additionally, the years could be incremented (or decremented) as high as desired, even though the min year was set to 2019 and the max year is set to 2040.
Video of tabbing through date spinners and where NaN appears after going past 31 for August and the year is set up to 2117

After, you can wrap around the days of the month. Also, 2019 shows up as the min year (with no previous year showing) and 2040 is the max year (with no next year showing).
Video of tabbing through date spinners and the date loops properly, with the year starting at 2019 and ending at 2040

The date picker spin button example would show NaN on all the dates if you tried to loop around to the first date. Also, despite there being a min and max year set, the year could be cycled through potentially infinitely beyond the min or max year.

This was due to the way the min and max values were being queried. The values are set on the DOM nodes for the node with the `spinbutton` role. However, the node that was being queried for those values was the parent node.

This change fixes the node used so that the initial values are set appropriately, and the spinners work as expected.
@mcking65
Copy link
Contributor

@mcking65
Copy link
Contributor

@backwardok, thank you very much for this fix!

Looks good to me -- looking at the code.

However, this is my first time testing this example in Safari on Mac ... I can't get it to work at all. I wonder if VoiceOver is somehow not reporting the value or accessible name correctly. The value is always Jun 30, 2019, no matter what I do.

I even tried passing through the up/down arrow keys with ctrl-opt-tab. When that didn't work, I made sure focus was on a spinner, turned off VoiceOver, then pressed up several times, then turned VO back on. VO still reported the same date for the group of spinners.

I tried activating the previous/next buttons with ctrl-opt-space. That did not change the date either.

It all works in Chrome with VoiceOver. Although, it is pretty painful that VO does not read the values as they change. You have to manually read them with ctrl-opt-w or ctrl-opt-f4. Amazing how broken VO is with these spinners.

I'll put this on the review agenda for September 3 meeting. I thought I'd be able to test and merge, but apparently this is not testable by a VoiceOver user -- at least not in Safari.

@backwardok
Copy link
Contributor Author

Hey @mcking65 - it's not testable in Safari because of the bug that I have a fix for in #1153 - you won't be able to test it in Safari until that one is merged (though you could theoretically merge the two together and see the full fix).

@mcking65
Copy link
Contributor

mcking65 commented Sep 1, 2019

In Safari, the label on the group is announced as "Choose a date, current value undefined" followed by the date, e.g., August 31, 2019. In Chrome, and with Windows screen readers, the label is correct, e.g., Choose a date, current value is Saturday August 31, 2019. Is the method being used to provide the name of the day of the week not supported in Safari?

Also, when using VoiceOver on the Mac in either Safari or Chrome, the value of the year stepper is not announced. When VO reads the day and month, it will say the value and then the name and then the role of stepper, e.g., "August Month Stepper". For the year, it just says "Year Stepper" and does not announce the value. Is there something different about the way the year value is communicated? I guess with the other 2 steppers we are using aria-valuetext and with the year we are not. So, perhaps VO has a bug where it does not announce the value of a spin button unless aria-valuetext is provided. If that is the case, this is a good test case for VO. Or, is there some other possible cause of this behavior?

@backwardok
Copy link
Contributor Author

@mcking65 I'm not sure, to be honest - I actually aimed to fix the Safari issue because I couldn't test the example with VO before to see what it sounded like. The Chrome behavior definitely seems better, but I don't know if that's an issue with VO + Safari or if there's another bug in the example.

@carmacleod
Copy link
Contributor

Tested in Windows 10 on Chrome, FF and Edge.

  • Keyboard works great in all 3 browsers.
  • JAWS and NVDA both work great in Chrome.
  • FF is getting the day of the week wrong (off by 1) in the hidden myDatepickerDate label.
    (for example, today is Tues Sept 10 2019, but the label says Monday September 10 2019)
    (probably need some special case code for FF?)
  • JAWS/Edge is eating the arrow keys in the spin buttons (probably a JAWS/Edge bug)
  • JAWS/Edge also doesn't announce the name of the parent group (again, probably a JAWS/Edge bug)
  • NVDA/Edge does spin the buttons on arrow keys, but it only speaks values from 0 - 100 (probably an NVDA/Edge bug)

@css-meeting-bot
Copy link
Member

css-meeting-bot commented Sep 24, 2019

The ARIA Authoring Practices (APG) Task Force just discussed Fix issue where spin button date and year are not initializing properly .

The full IRC log of that discussion <carmacleod> github: https://github.com//pull/1154
<carmacleod> mck: works with month and day, but not year?
<carmacleod> mck: is this a bug in the example, or a safari bug?
<carmacleod> jon: spinbutton doesn't work in vo
<carmacleod> mck: MacOS safari does work with VO - it just doesn't work with VO commands
<carmacleod> jon: could use aria-valuetext instead of aria-valuenow
<carmacleod> mck: need to report VO bug
<carmacleod> jon: I can look into this

Copy link
Contributor

@mcking65 mcking65 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@backwardok, This looks good to go.

I've verified results from @carmacleod. I think the Edge issues are screen reader problems, not example code problems. And, the Firefox problems are present in the published version. So, they are not related to your changes. I raised #1188 for the Firefox group label problem.

@mcking65 mcking65 added bug Code defects; not for inaccurate prose Example Page Related to a page containing an example implementation of a pattern labels Sep 28, 2019
@mcking65 mcking65 added this to the 1.2 Third Working Draft milestone Sep 28, 2019
@mcking65 mcking65 changed the title Fix issue where spin button date and year are not initializing properly Spin Button Date Picker Example: Fix incrementing and decrementing past limits Sep 28, 2019
@mcking65 mcking65 merged commit 559f473 into w3c:master Sep 28, 2019
michael-n-cooper pushed a commit that referenced this pull request Sep 28, 2019
Spin Button Date Picker Example: Fix incrementing and decrementing past limits (pull #1154)

Fixes the following bugs in the date picker spin button example:
1. The day would show NaN when decrementing past the min value or incrementing past the max value.
2. The year would show NaN when setting to min or max with home or end keys.
3. The year could be set beyond the min or max year by incrementing or decrementing.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Code defects; not for inaccurate prose Example Page Related to a page containing an example implementation of a pattern
Development

Successfully merging this pull request may close these issues.

4 participants