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

Clarify VoiceOver text for dates selected as start-date and end-date #1501

Merged
merged 3 commits into from
Jan 10, 2019

Conversation

nkinser
Copy link
Contributor

@nkinser nkinser commented Jan 7, 2019

This PR adds clarity to the VO text for dates selected as check-in and check-out. Previously, the VO text for all selected dates was "Selected. ${date}", and I have updated it so that if the date is selected for check-in it is "Selected for check-in. ${date}" and if it is selected for check-out it is "Selected for check-out. ${date}".

jira ticket: https://jira.airbnb.biz/browse/PRODUCT-56423

@coveralls
Copy link

coveralls commented Jan 7, 2019

Coverage Status

Coverage increased (+0.05%) to 85.185% when pulling e9197f0 on nkinser:nk--clarify-VO-for-selected-dates into bc0f29a on airbnb:master.

@nkinser
Copy link
Contributor Author

nkinser commented Jan 8, 2019

@majapw @ljharb Can you PTAL when you get the chance? Thanks!

@@ -36,7 +38,13 @@ export default function getCalendarDaySettings(day, ariaLabelFormat, daySize, mo

let ariaLabel = getPhrase(chooseAvailableDate, formattedDate);
if (selected) {
ariaLabel = getPhrase(dateIsSelected, formattedDate);
if (modifiers.has('selected-start')) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I add a null check here for dateIsSelectedAsCheckin and dateIsSelectedAsCheckout, so that it doesn't throw an error if someone using react-dates is overriding the default phrases and hasn't added these two phrases?

Copy link
Collaborator

Choose a reason for hiding this comment

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

That might be a good mitigating factor.

Copy link
Collaborator

@majapw majapw left a comment

Choose a reason for hiding this comment

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

Looks great! Would love to use startdate/enddate instead of checkin/checkout but other than that looks good!

@@ -128,6 +134,8 @@ export const SingleDatePickerPhrases = {
chooseAvailableDate,
dateIsUnavailable,
dateIsSelected,
dateIsSelectedAsCheckin,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think the SDP needs these as it only has one date

@@ -30,6 +30,8 @@ const chooseAvailableEndDate = ({ date }) => `Choose ${date} as your check-out d
const chooseAvailableDate = ({ date }) => date;
const dateIsUnavailable = ({ date }) => `Not available. ${date}`;
const dateIsSelected = ({ date }) => `Selected. ${date}`;
const dateIsSelectedAsCheckin = ({ date }) => `Selected for check-in. ${date}`;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we s/checkin/startdate and s/checkout/enddate for consistency with this library? We can use checkin/checkout at airbnb but it'd be good to keep the nomenclature consistent within this library.

@@ -36,7 +38,13 @@ export default function getCalendarDaySettings(day, ariaLabelFormat, daySize, mo

let ariaLabel = getPhrase(chooseAvailableDate, formattedDate);
if (selected) {
ariaLabel = getPhrase(dateIsSelected, formattedDate);
if (modifiers.has('selected-start')) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

That might be a good mitigating factor.

Copy link
Collaborator

@majapw majapw left a comment

Choose a reason for hiding this comment

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

Lookin' good! :shipit:

@majapw majapw merged commit 3efefd0 into react-dates:master Jan 10, 2019
@majapw majapw changed the title Clarify VoiceOver text for dates selected as check-in and check-out Clarify VoiceOver text for dates selected as start-date and end-date Jan 15, 2019
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

3 participants