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

Accessibility : Buttons without text or aria label, texts are all in divs only.. #161

Closed
eddr opened this issue Jul 20, 2023 · 17 comments
Closed
Labels
enhancement New feature or request

Comments

@eddr
Copy link

eddr commented Jul 20, 2023

Hi

Checking the demo site, it seems that the content is not accessible even on the basic level:

  • navigation buttons without labels/texts
  • all events' texts are inside divs only
  • and in general, no use of semantic HTML..

Is it on purpose?

Thanks

@vkurko
Copy link
Owner

vkurko commented Jul 20, 2023

Is it on purpose?

What answer do you expect to hear by asking this question? 😄

If you have suggestions for improving the calendar, I'm open to consider them, just write them in a constructive way, please.

@eddr
Copy link
Author

eddr commented Jul 20, 2023

Hi Vladimir

It was not my intention at all and I apologize. I was just trying to write what I see as lacking a short list so it will be readable..
Communicating in text can be tricky sometimes

  • And I was also wondering if I'm missing anything, like a roadmap to accessibility or simple JS hooks and customization that allow me to add text

  • Not an expert, but for the events I would probably consider using HTML tags like article with a p inside perhaps

  • and add aria-label attributes to all buttons

Thanks

@vkurko
Copy link
Owner

vkurko commented Jul 20, 2023

I agree, there are a11y issues. Thanks for the suggestions, I'll try to implement them in the next updates.

@vkurko
Copy link
Owner

vkurko commented Jul 26, 2023

Version 1.5.1 fixed some issues with a11y:

  1. All buttons that do not have accessible name now have the aria-label attribute with the appropriate text
  2. Show more and Close buttons are now keyboard accessible
  3. I've put off replacing <div> with <article>+<p> for now because I haven't seen anyone use this approach in other calendars (if you have examples, please let me know). Also using <article> generates A11y: Non-interactive element <article> should not be assigned mouse or keyboard event listeners. warning in Svelte.

@eddr
Copy link
Author

eddr commented Jul 31, 2023

That's amazing (: Thank you

About 3 - I agree that it was perhaps too specific, but since the event title and content are texts elements with semantic meaning, it is better to use a different element if possible. I'm not sure why others don't take it into account
Wrote more in the list below

List:

  1. Using "nav" HTML element for containers of navigation buttons
  2. All the "ec-time"/"ec-day" class elements : I think can also be a "time" element
  3. Same for date ranges - if possible - with two "time" elements for the start and end date-time values. In addition, perhaps user "aria-label" for the container. Something like that:

From: January 1, 2023 To: December 31, 2023

  1. About the event title/content/container:
  • I'm not sure about "article" but it should be something different than "div".
    For the "ec-event-title" class elements, I'd say at least an "h" element of some sort or a "p" with an appropriate aria-role ( user configurable if it's not too much to ask )

"The Events Calendar" plugin uses specific elements - article, h, p:
https://demo.theeventscalendar.com/events/month/

The Guardian uses fullcalendar as well ( apparently ), and the inner elements HTML are p and h for title and content
https://www.theguardian.com/sport/series/the-calendar

  • Are the events anchors? If they are, why not an "a" element?

What do you think?

@vkurko
Copy link
Owner

vkurko commented Aug 7, 2023

Thanks for the examples. Indeed someone is using <article> for events. I will try to improve a11y in future releases.

@eddr
Copy link
Author

eddr commented Aug 7, 2023

Amazing, thanks

@vkurko vkurko added the enhancement New feature or request label Aug 22, 2023
@vkurko
Copy link
Owner

vkurko commented Dec 9, 2023

@eddr Sorry, this took a while. I released version 2.5.0 which contains:

  1. Events use <article> and <h4> tags
  2. Events are now accessible from the keyboard
  3. Day names are table column headers with the correct aria-label (added dayHeaderAriaLabelFormat option)
  4. The toolbar uses the <nav> tag

Please test it and tell me what you think.

@eddr
Copy link
Author

eddr commented Dec 11, 2023

Amazing work

@vkurko
Copy link
Owner

vkurko commented Dec 11, 2023

Great. A small clarification - events are accessible from the keyboard only when the eventClick hook is defined for events.

Let me close this ticket and ask that a new issue be opened for future improvements.

Thanks.

@vkurko vkurko closed this as completed Dec 11, 2023
@newpen
Copy link

newpen commented Apr 24, 2024

Thanks a lot! But I still do not get aria-label for the ec-prev and ec-next buttons. Do I need to enable them somewhere?

@vkurko
Copy link
Owner

vkurko commented Apr 25, 2024

Thanks a lot! But I still do not get aria-label for the ec-prev and ec-next buttons. Do I need to enable them somewhere?

No, it should work out of the box. Are you accidentally overriding the buttonText option?

@newpen
Copy link

newpen commented Apr 25, 2024

Thanks a lot! But I still do not get aria-label for the ec-prev and ec-next buttons. Do I need to enable them somewhere?

No, it should work out of the box. Are you accidentally overriding the buttonText option?

Oh Yes, I am overriding the 'today' button because I want it to display in a different language. I am not overriding the next/prev buttons. Is there any way I can keep the aria-labels for the navigation buttons while displaying custom text for the today button? Thanks a lot!

FYI, I have this:
buttonText: {
today: 'my custom text'
},

@vkurko
Copy link
Owner

vkurko commented Apr 25, 2024

I have this:
buttonText: {
today: 'my custom text'
},

The current strategy for merging options for each view is as follows:

viewOptions = defaultCommonOptions < defaultViewOptions < userCommonOptions < userViewOptions

where xxxCommonOptions are those options that are intended for all views, and xxxViewOptions are respectively intended for a specific view and are passed inside the views property. < means that the options on the right overwrite the options on the left.

As you can see, by passing the buttonText as an object, you are essentially setting the same object (with empty prev and next properties) for all views.

When the buttonText is given as a function, instead of overwriting, this function will be called and the object from the previous merge step will be passed to it. Therefore, in your case, the option with the function should work.

So try

buttonText: function (text) {
  text.today = 'my custom text';
  return text;
}

@newpen
Copy link

newpen commented Apr 25, 2024

It works! Thanks a lot! But I have another question. Is there any way to set custom aria-labels too? It's because sometimes I may need those labels in another language as well. Thank you!

@vkurko
Copy link
Owner

vkurko commented Apr 25, 2024

Sure. There are prev and next fields for that. See buttonText.

buttonText: function (text) {
  text.today = 'my custom text';
  text.prev = 'my custom text';
  text.next = 'my custom text';
  return text;
}

@newpen
Copy link

newpen commented Apr 25, 2024

Thanks a lot!

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

No branches or pull requests

3 participants