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

First pass at making Pikaday accessibile #522

Merged
merged 13 commits into from May 20, 2016
Merged

Conversation

Robdel12
Copy link
Contributor

First I'd like to give all of the credit to @mrenty. He did pretty much all of the hard work on this and I came in to push it over the line.

Today is Global Accessibility Awareness Day and one of the things I really wanted to do for GAAD was make Pikaday accessibile. With this datepicker being the go to for the majority of the dev community it's important to me that this plugin is accessibile.

Here's a video example of what it's like to use this PRs Pikaday with a screen reader: https://www.youtube.com/watch?v=iZhi4nQP5kI

Here's what it's like to use pikaday with a screen reader today:
https://youtu.be/mrUwra6C34U

CC: @dbushell / @rikkert

@mrenty
Copy link

mrenty commented May 19, 2016

Nice job @Robdel12! Hope it merges, accessibility is not just a feature it's just basic requirement because it helps people navigate the web that we're building.

@rikkert rikkert merged commit 3f4b9c9 into Pikaday:master May 20, 2016
@rikkert
Copy link
Member

rikkert commented May 20, 2016

This is great, thank you!

@Robdel12
Copy link
Contributor Author

@rikkert thank you for merging so quickly! I should have mentioned this PR was an extension of #458. So that one could be closed now :)

JakoD added a commit to JakoD/Pikaday that referenced this pull request Jun 6, 2016
First pass at making Pikaday accessibile (Pikaday#522)
@@ -922,6 +981,8 @@
if (typeof this._o.onDraw === 'function') {
this._o.onDraw(this);
}
// let the screen reader user know to use arrow keys
this._o.field.setAttribute('aria-label', 'Use the arrow keys to pick a date');
Copy link
Contributor

Choose a reason for hiding this comment

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

This errors when a field isn't bound.

@hvb-pla
Copy link

hvb-pla commented Aug 4, 2016

hey guys, thank you for your efforts! unfortunately when changing the date using the arrow keys the changed value/date does not get announced by voiceover. maybe something changed in the behaviour of voiceover? i am using chrome (51.0.2704.106) on mac os x 10.11.5 and used the latest master branch to test this.

@Robdel12
Copy link
Contributor Author

Robdel12 commented Aug 4, 2016

@hvbe Chrome might be the issue. I've noticed lots of issues with Chrome & aria-live. It sucks it doesn't work as it should in chrome but "thankfully" the combos in the a11y world that are heavily used are:

OSX - Safari - VoiceOver
Windows - Firefox - NVDA
Windows - IE - JAWS
iOS - Safari - VoiceOver

As much as it pains me to say this... try in Safari?

@hvb-pla
Copy link

hvb-pla commented Aug 4, 2016

@Robdel12 thanks for your reply. indeed, in safari it seems to be working without issues. let's just say that the chrome + voiceover combo is an edge case :)

@dannylivewire
Copy link

From what I can tell... there is a bug happening because the listener is attached to the document at - https://github.com/dbushell/Pikaday/blob/master/pikaday.js#L629

What's going on, is that if you are on another element or anything else on the page, and use the arrow keys (to open a drop down or something else on the page).. It will also adjust the date field's value globally. ... Or that's what it seems like is going on at least.

@mrenty
Copy link

mrenty commented Sep 12, 2018

From what I can tell... there is a bug happening because the listener is attached to the document at - https://github.com/dbushell/Pikaday/blob/master/pikaday.js#L629

What's going on, is that if you are on another element or anything else on the page, and use the arrow keys (to open a drop down or something else on the page).. It will also adjust the date field's value globally. ... Or that's what it seems like is going on at least.

It looks like there's missing an extra check missing to see if self.el.contains(document.activeElement), maybe someone can add that in an existing PR like @kunal999?

@mrleblanc101
Copy link

This is not accessible at all... VoiceOver doesn't even read the date.
The aria-live is broken. It just reads the header instead of the currently focused date.
Capture d’écran, le 2019-06-18 à 15 17 30

@mrenty
Copy link

mrenty commented Jul 19, 2019

This is a known issue (see #776) and seems to be regression caused by a PR merged after this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants