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

add isDateDisabled prop and styling #1135

Merged
merged 13 commits into from
Mar 20, 2023
Merged

add isDateDisabled prop and styling #1135

merged 13 commits into from
Mar 20, 2023

Conversation

mayank99
Copy link
Contributor

@mayank99 mayank99 commented Mar 14, 2023

Changes

  • Added disabled text styling for aria-disabled and removed extra hover interactions.
  • Added isDateDisabled prop which takes a function which must return a boolean. If true, the date will have aria-disabled='true' and will also disable click/Enter handlers.
  • Previous/next month and year buttons will be automatically disabled when the last or first date in it is disabled.

Testing

Tested in storybook. Will test more permutations in playgrounds tomorrow.

Added new unit test, visual tests.

Docs

Added changeset, new story and jsdoc for new prop.

@mayank99 mayank99 self-assigned this Mar 14, 2023
@mayank99 mayank99 linked an issue Mar 14, 2023 that may be closed by this pull request
@mayank99 mayank99 marked this pull request as ready for review March 16, 2023 21:20
@mayank99 mayank99 requested a review from a team as a code owner March 16, 2023 21:20
@mayank99 mayank99 requested review from a team, FlyersPh9, LostABike and gretanausedaite and removed request for a team March 16, 2023 21:20
Copy link
Contributor

@gretanausedaite gretanausedaite left a comment

Choose a reason for hiding this comment

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

Code looks good

Is it possible to make screen reader announce that it's a date?
Also, saying "selected" when date is not selected.
image

@mayank99
Copy link
Contributor Author

Is it possible to make screen reader announce that it's a date? Also, saying "selected" when date is not selected.

That is outside the scope for this PR. I think the pattern was copied from APG which also only announces the current date.

Also it doesn't say "selected" for me. When using VoiceOver, make sure you're testing in Safari.

For the purposes of this PR, all that matters is that disabled state is conveyed, which is indeed working correctly. VoiceOver announces it as "dimmed".

@mayank99
Copy link
Contributor Author

Is it possible to make screen reader announce that it's a date?

That is outside the scope for this PR. I think the pattern was copied from APG which also only announces the current date.

Actually this is not true. APG uses a table, which first announces the name of the column ("Tuesday") and subsequently only announces the day when you're in the same column.

@veekeys Do you remember the research/sources used for our datepicker?

@veekeys
Copy link
Member

veekeys commented Mar 20, 2023

APG

Are you asking only about the a11y part? I want to say that I did check some of the attributes from W3 as usual, but the DOM structure is different so not all of those were applied. Date picker was never tested thoughtfully with the screen readers AFAIK.

@mayank99
Copy link
Contributor Author

We'll need to test all our components in at least NVDA and VoiceOver. I've created a separate issue to track that: #1148


I think this PR should be good to merge now. I've tested visuals, mouse and keyboard behavior for a few different permutations in the vite playground. Here's the code if anyone else feels like testing:

Disable all dates before today
import * as React from 'react';
import { DatePicker } from '@itwin/itwinui-react';

let today = new Date();
today = new Date(today.getFullYear(), today.getMonth(), today.getDate());

const App = () => {
  const [dateValue, setDateValue] = React.useState<Date>(today);
  return (
    <>
      <DatePicker
        date={dateValue}
        onChange={(date) => setDateValue(date)}
        isDateDisabled={(date) => date < today}
      />
      <pre>{dateValue?.toLocaleDateString()}</pre>
    </>
  );
};

export default App;
Two date pickers (range) with connected min/max values
import * as React from 'react';
import { Button, DatePicker } from '@itwin/itwinui-react';

let today = new Date();
today = new Date(today.getFullYear(), today.getMonth(), today.getDate());

const App = () => {
  const [fromValue, setFromValue] = React.useState<Date>();
  const [toValue, setToValue] = React.useState<Date>();

  return (
    <>
      <DatePicker
        date={fromValue}
        onChange={(date) => setFromValue(date)}
        isDateDisabled={(date) => (toValue ? date > toValue : false)}
      />
      <DatePicker
        date={toValue || fromValue}
        onChange={(date) => setToValue(date)}
        isDateDisabled={(date) => (fromValue ? date < fromValue : false)}
      />

      <pre>
        {fromValue?.toLocaleDateString()} - {toValue?.toLocaleDateString()}
      </pre>
      <Button
        onClick={() => {
          setFromValue(undefined);
          setToValue(undefined);
        }}
      >
        Clear
      </Button>
    </>
  );
};

export default App;
Only allow next two weeks
import * as React from 'react';
import { DatePicker } from '@itwin/itwinui-react';

const today = new Date();

const App = () => {
  const [dateValue, setDateValue] = React.useState<Date>(today);
  return (
    <>
      <DatePicker
        date={dateValue}
        onChange={(date) => setDateValue(date)}
        isDateDisabled={(date) => {
          if (date < today) {
            return true;
          }

          const daysDiff =
            Math.abs(
              Date.UTC(date.getFullYear(), date.getMonth(), date.getDate()) -
                Date.UTC(
                  today.getFullYear(),
                  today.getMonth(),
                  today.getDate(),
                ),
            ) /
            (1000 * 60 * 60 * 24);
          if (daysDiff > 14) {
            return true;
          }

          return false;
        }}
      />
      <pre>{dateValue?.toLocaleDateString()}</pre>
    </>
  );
};

export default App;

@mayank99 mayank99 added this pull request to the merge queue Mar 20, 2023
@mayank99 mayank99 merged commit 61f4429 into main Mar 20, 2023
@mayank99 mayank99 deleted the mayank/disabled-dates branch March 20, 2023 15:18
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.

DatePicker: disable some dates
5 participants