-
Notifications
You must be signed in to change notification settings - Fork 34
Add partial check-in/check-out indicators and screen reader accessibility features to datepicker (WooExt-330) #550
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
base: trunk
Are you sure you want to change the base?
Conversation
- Introduced visually hidden text for screen readers to enhance accessibility for datepicker cells. - Implemented functions to format and add accessible text for fully booked and partially available dates. - Updated styles for fully booked dates to include visual indicators (crosses) for better user experience. - Ensured that screen reader text is re-added after date selection triggers a refresh.
|
This is linked to GitHub issue #551. |
|
This is linked to GitHub issue #552. |
|
This is linked to GitHub issue #553. |
iamdharmesh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR @m-muhsin. Added few questions could you please check. Thanks
src/js/booking-form.js
Outdated
| */ | ||
| const formatDateForScreenReader = ($cell) => { | ||
| const $dayElement = $cell.find('span, a').first(); | ||
| const day = $dayElement.contents().first().text().trim(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about add formatted date attribute and use that directly instead of parsing content and generate it?
… generation for booked dates.
iamdharmesh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the changes @m-muhsin. Added note around formattedDate not getting added. Could you please help to check? Also, Eslint action is getting failed, would be great if you can fix the reported errors there as well. Thanks.
…date formatting by robustly retrieving date components, and refine `addAccessibleText` targeting.
iamdharmesh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, correct. I have updated accordingly. Thank you. |
|
@m-muhsin is there is anything pending here or did you address all the feedback above? If yes, can you re-request review to @iamdharmesh so this can be progressed further? |
…visual indicators and updated gradients for availability representation.
|
Hi @vikrampm1. We had a discussion around this in yesterday's call. We decided to implement it like this:
I have done it likewise now. Thanks! |
|
@m-muhsin it would be great to get these addressed soon so we can move this to the next step. Thanks! |
Co-authored-by: Dharmesh Patel <[email protected]>
|
@iamdharmesh I have now incorporated your feedback. Thanks! CC: @vikrampm1 |
|
Thanks for the changes @m-muhsin. Changes looks good. However, I noticed that accessibility text not getting added on months other than current. Example: It works fine for current month (Dec) but not for the January |
|
@m-muhsin Reminder for addressing this. |
|
Thanks for the reminder, @iamdharmesh. I was trying to replicate it but could not. I even checked for 2027 just in case it was for the following year.
|
|
Note: As discussed over call UI part is work well but accessibility text not getting added on months other than current. Thanks. |
…r improved accessibility.
…o that it applies to other months than the current month alone.
|
@iamdharmesh I have now added even listeners to change of month. So, the last reported issue is now sorted. Thanks! |
| 'change', | ||
| '.ui-datepicker-month, .ui-datepicker-year', | ||
| function () { | ||
| setTimeout(() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this setTimeout? If we don't have proper event to tag this then we might look into fix the issues in bookings itself if that's more solid way to fix this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was required for when testing with a throttled network. I have removed it now. Shall we address it upstream if it comes again?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@iamdharmesh as discussed I added the setTimeout back along with a comment explaining it.
|
Thanks for fixing it @m-muhsin. Added one question to get some information. |
|
@iamdharmesh as mentioned, in this comment, can we address it in upstream if it comes up again? I removed the setTimeout which is required for slower networks. |
…n reader text for improved accessibility.
|
@iamdharmesh I felt better to add the setTimeout so did it with a comment explaining it. |


All Submissions:
Changes proposed in this Pull Request:
This PR enhances the accessibility and visual clarity of the accommodation booking datepicker by:
Accessibility Improvements:
addAccessibleText()function to dynamically add screen reader announcements to date cellsformatDateForScreenReader()function to format dates in a human-readable format (e.g., "January 15, 2025, Monday") for screen reader usersVisual Enhancements:
::beforeand::after)Technical Implementation:
.screen-reader-textCSS class following WordPress accessibility standards for visually hidden contentFixes the following accessibility issues:
WooExt-330.
Closes #553
Closes https://linear.app/a8c/issue/WOOACBK-76/tracking-issue-for-pr-550-add-screen-reader-accessibility-features-to
Steps to test the changes in this Pull Request:
Changelog entry