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

week numbers #53

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

week numbers #53

wants to merge 7 commits into from

Conversation

WickyNilliams
Copy link
Owner

@WickyNilliams WickyNilliams commented Jun 14, 2024

  • adds show-week-numbers option to date/range/multi components
  • adds weeknumber slot to month component
  • adds <col>s plus css parts for each column in month
  • adds weekday part for day headings, weeknumber part for week number heading

Todo

  • test how week numbers are announced in SRs

@raphaelokon
Copy link

@WickyNilliams Hi there. Is there anything we could help with to get the show-week-numbers feature released? Cheers for the great component btw.

@WickyNilliams
Copy link
Owner Author

I just need to merge and release really. Not sure why I didn't at the time, since the feature is complete.

I haven't had much time recently as I've had a lot going on. But I'm planning on putting some hours in later this week. I'll ping you when it's released :)

In the meanwhile, feel free to take a look at the code and give any feedback!

@raphaelokon
Copy link

Thanks @WickyNilliams

I already did! Looks really good, it is a breeze to use Cally.

@raphaelokon
Copy link

@WickyNilliams One thing that might be a tricky one is that if the week-numbers are structurally part of the table, they will be part of the navigation of assistive technologies like NVDA and VoiceOver. Since the week-numbers do not take part in the date selection process this might confuse screenreader users. What do you think?

@WickyNilliams
Copy link
Owner Author

It should announce "30th December, Monday, week 52" (order may not be correct, I'm going from memory), which I think is fine?

The week will only be announced when you change week (move focus vertically). Similar to how day is only announced when you change day (move focus horizontally). So it shouldn't be overly noisy. If you have chosen to show the week number, it feels like you'd want this announced else why show the week number?

@raphaelokon
Copy link

That sounds reasonable. I just wanted to make the point that if the week-numbers are part of the table, the table could be announced with one additional columnwhich might confuse the user. This is just wild guessing tho as I do not have the rendered markup in front of me.

@WickyNilliams
Copy link
Owner Author

Ah I see what you mean. The week number cells are configured as headers, rather than a regular cell. So I hope it should be clear, especially if devs give a reasonable title to the column.

I'll make sure to do some additional screen reader testing before merge

@raphaelokon
Copy link

This is a bit off-topic, but related to weeks → Do you think it would be feasible to let the shortDayName be configurable between 'narrow' | 'short'? → https://github.com/WickyNilliams/cally/blob/main/src/calendar-month/useCalendarMonth.ts#L21

@WickyNilliams
Copy link
Owner Author

following up: I made some tweaks over the weekend to improve how weeks get announced. I need to do more testing with various screen readers to see how they all fare

@WickyNilliams
Copy link
Owner Author

@raphaelokon that might be nice, can you create a new issue for that

@raphaelokon
Copy link

@WickyNilliams#66

@raphaelokon
Copy link

@WickyNilliams Do you need any testing help with the week-numbers? I also created an issue re configuring the weekname abbreviation.

@WickyNilliams WickyNilliams changed the title Feature/week numbers week numbers Nov 3, 2024
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.

2 participants