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

fix: add missing accessibility attributes, fixes #233 #296

Closed

Conversation

ghiscoding
Copy link
Contributor

@ghiscoding ghiscoding commented Aug 18, 2024

fixes #233

  • add aria-label to a few elements
    • days (I had to add the "long" read of weekday from locale, perhaps we could use it for short too with a substring of 3 chars?)
    • months
    • years
    • arrows previous/next for both year/month (I went with a regex pattern to read an extra attribute to component as in <#ArrowPrev [year] /> which year is the optional attribute which is then used to read self.ariaLabels.nextMonth)
  • add aria-selected to all dates and also month and/or year picker

I think that this should be enough to make the picker more accessible and close the associated issue :)

(self.locale.weekday as string[]).push(capitalizeFirstLetter(weekday));
(self.locale.longWeekday as string[]).push(capitalizeFirstLetter(longWeekday));
Copy link
Contributor Author

@ghiscoding ghiscoding Aug 18, 2024

Choose a reason for hiding this comment

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

another option might be to keep both long/short weekday in a single array of objects, but that might be a breaking change, so perhaps acceptable for 3.0 roadmap?

const weekday = [{ long: 'Monday', short: 'Mon' }, ...]

or yet another option would be to keep only the long version and then do a substring of the first 3 chars, but I don't know if that is valid for all locale? maybe not

@@ -3,8 +3,9 @@ import getComponent from '@scripts/helpers/getComponent';

export const DOMParser = (self: VanillaCalendar, template: string) => (
template.replace(/[\n\t]/g, '').replace(/<#(?!\/?Multiple)(.*?)>/g, (_, p1) => {
const component = getComponent(p1.replace(/[/\s\n\t]/g, ''));
const html = component ? component(self) : '';
const [__, comp, attribute] = /(.*)\s?\[(.*)\].*/g.exec(p1) || [];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm sure this regex could be improved and perhaps merged with the regex just above on the previous line but I played it safe and couldn't figure out the best regex to use, feel free to change. The regex is to extract any optional attribute that I added to some of the component like <#ArrowPrev [month] /> or <#ArrowPrev [year] />

@uvarov-frontend
Copy link
Owner

uvarov-frontend commented Aug 18, 2024

There is definitely no need to create a longWeekday array, it will be enough to create a parameter like this settings.visibility.monthShort, only for weeks. And always insert a long name into the date attributes.
In fact, tasks #233 and #260 are very related, and I plan to completely rewrite the use of css classes to data and aria attributes, solving two problems at once. Therefore, I will not be able to upload this PR, but thank you for your contribution, I will use your ideas from this PR when I write code, thank you!

@ghiscoding
Copy link
Contributor Author

All good, I just provided this code as a guideline, feel free to change it all the way you want for 3.0. Another thing that we could potentially do in the future, though it's just an idea, we could listen to keyboard arrows to allow selecting a date without ever touching the mouse. Below is an example from Flatpickr (the previous date picker I was using)

brave_B97wPnzfvQ

@uvarov-frontend
Copy link
Owner

All good, I just provided this code as a guideline, feel free to change it all the way you want for 3.0. Another thing that we could potentially do in the future, though it's just an idea, we could listen to keyboard arrows to allow selecting a date without ever touching the mouse. Below is an example from Flatpickr (the previous date picker I was using)

brave_B97wPnzfvQ

Yes, there will be control with the help of arrows, too, it was in the plans.

@uvarov-frontend
Copy link
Owner

Implemented in v3.0.0 #293

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.

Accessibility
2 participants