Correct calendar high contrast #6938

jhellesen · 2018-11-01T22:18:00Z

Pull request checklist

Addresses an existing issue: Fixes #0000
Include a change request file using $ npm run change

Existing calendar component has several issues in high contrast

Many buttons lack an active state different from hover state
Day buttons lacks clear hover / active states, especially as related to current day and selected day
No visual indicator current month / year
Hover / active state incorrect
Hover / active states for week / month are inconsistent from non-high contrast
Hover / active states for week / month have visual clipping
Rest color for 'Go to today' button

Focus areas to test

Should be no changes to non-high contrast other than correcting (regression) the font-size of the day buttons
The rest, hover and active states for the calendar day buttons when the date range type is day, week or month
The rest, hover and active states for the month / year buttons, the previous / next buttons

Microsoft Reviewers: Open in CodeFlow

… dates outside of the date boundary (minDate, maxDate) when DateRangeType.Week is used.

Divs were recently changes to buttons for accessibility and they inherited a button style for the font line height causing the text to shift downwards significantly inside the cells. Switching back to the old default.

…out of range dates when DateRangeType is Month

…c-react

…e updated high contrast rest / hover / active pattern.

…ntrast rest / hover / active pattern.

…ive high contrast states for the current day and selected day.

… active high contrast states.

1) Update calendar 'Go to today' button to appear as a link in high contrast mode 2) Update calendar day week and month selection types to conform with the updated rest / hover / active high contrast pattern 3) Fix calendar day selection bug where day text wasn't visible in high contrast

1) Fix issue where 'Go to today' link was broken 2) Fix issue where high contrast month hover state borders were not rounded 3) Fix issue where text in high contrast was shifting location for month, week and Month/Year hover states

…c-react

…the buttons inherit the font-size.

…c-react

natalieethell · 2018-11-06T21:37:06Z

Hey @jhellesen it looks like your snapshot tests failed. Is changing "Go to today" to " Go to today" (an extra space in front) intentional?

…nd weight

jhellesen · 2018-11-06T22:45:22Z

Hey @jhellesen it looks like your snapshot tests failed. Is changing "Go to today" to " Go to today" (an extra space in front) intentional?

No, and it's fixed now. Thanks.

natalieethell · 2018-11-06T23:29:33Z

Cool, thanks! It looks like the newest snapshot failure is due to the addition of disabled={true} in the "Renders simple calendar correctly" test. Do you know what could be causing this?

…y' button

jhellesen · 2018-11-06T23:37:14Z

Cool, thanks! It looks like the newest snapshot failure is due to the addition of disabled={true} in the "Renders simple calendar correctly" test. Do you know what could be causing this?

Sorry - that was an intentional change. I've pushed out an update to the snapshot.

lorejoh12 · 2018-11-07T23:15:32Z

packages/office-ui-fabric-react/src/components/Calendar/Calendar.tsx

+
+    if (goTodayEnabled && navigatedDayDate && selectedDate) {
+      goTodayEnabled =
+        navigatedDayDate.getFullYear() !== selectedDate.getFullYear() || navigatedDayDate.getMonth() !== selectedDate.getMonth();


You probably meant to use "today" date rather than "selected" date.

Bug happens when you navigate to previous month, then select a day in the middle of the month, the go to today button gets disabled and it shouldn't

related bug, you're using getMonth() but that doesn't cover what days are actually visible - if you select a visible day at the beginning or end of the range that's in the previous month, the button gets enabled again even if it can't do anything.

natalieethell · 2018-11-07T23:53:18Z

In the deploy demo link, I noticed that if you select a date in a different month than the current month, and then re-open the DatePicker, "Go to today" is disabled. Is that expected?

If I click on a month on the month picker, "Go to today" is enabled again.

…c-react

aneeshack4 · 2018-11-19T21:45:40Z

@jhellesen Hello! I see that this PR has been open for quite some time, do you have any plans on fixing the screener visual regressions so that this can be merged? Please let us know if we can answer any questions!

lorejoh12 · 2018-11-26T21:36:10Z

@jhellesen Hello! I see that this PR has been open for quite some time, do you have any plans on fixing the screener visual regressions so that this can be merged? Please let us know if we can answer any questions!

These changes are actually intentional, not regressions. We had an existing bug for a while with the font inheritance being broken, so we actually need the changes to be approved. Our calendar designer doesn't have access to approve them- who do we need to talk to?

natalieethell · 2018-11-26T21:38:27Z

@lorejoh12 Ben (@betrue-final-final) should be able to help you with those.

msft-github-bot · 2018-11-29T19:36:34Z

🎉office-ui-fabric-react@v6.108.0 has been released which incorporates this pull request.:tada:

Handy Links:

jhellesen added 15 commits October 2, 2018 14:48

Correct Calendar component to not allow highlighting and selecting of…

e20f680

… dates outside of the date boundary (minDate, maxDate) when DateRangeType.Week is used.

Correct text downward shift.

d24c5d9

Divs were recently changes to buttons for accessibility and they inherited a button style for the font line height causing the text to shift downwards significantly inside the cells. Switching back to the old default.

Add change file

0bfc6eb

Fix issue where days would show their hover state when hovering over …

6b3a29a

…out of range dates when DateRangeType is Month

Merge branch 'master' of https://github.com/OfficeDev/office-ui-fabri…

466b9fd

…c-react

Merge branch 'master' of https://github.com/OfficeDev/office-ui-fabri…

3574f94

…c-react

Update Month, Previous / Next arrows, Year / Month buttons to have th…

33b3d1a

…e updated high contrast rest / hover / active pattern.

Update calendar day buttons (left pane) to follow the updated high co…

1121f74

…ntrast rest / hover / active pattern.

Update calendar day buttons to support the updated rest / hover / act…

4b98f97

…ive high contrast states for the current day and selected day.

Update calendar selected month to support the updated rest / hover /…

b6ee846

… active high contrast states.

Merge branch 'master' of https://github.com/OfficeDev/office-ui-fabri…

58c39e7

…c-react

Correct the font-size (regression) for calendar day items by letting …

feb728c

…the buttons inherit the font-size.

Merge branch 'master' of https://github.com/OfficeDev/office-ui-fabri…

143d66b

…c-react

jhellesen requested a review from a team as a code owner November 1, 2018 22:18

jhellesen added 4 commits November 2, 2018 15:17

Merge branch 'master' of https://github.com/OfficeDev/office-ui-fabri…

d6aa27d

…c-react

Disable the 'Go to today' button when it has no effect.

37f08ba

Merge branch 'master' of https://github.com/OfficeDev/office-ui-fabri…

a53a862

…c-react

Convert the calendar 'Go to today' back to a button from a link'

2e4a83a

Fix styling for calendar day text by allowing them to inherit color a…

7055c28

…nd weight

Update Calendar snapshot to include disabled property for 'Go to toda…

41c2418

…y' button

lorejoh12 reviewed Nov 7, 2018

View reviewed changes

natalieethell assigned jhellesen Nov 8, 2018

Merge branch 'master' of https://github.com/OfficeDev/office-ui-fabri…

945b9e1

…c-react

jhellesen added 2 commits November 8, 2018 16:33

Correct calendar logic determining when 'Go to today' should be enabled

1152044

Update snapshots for corrected 'Go to today' disable logic

9619934

aneeshack4 added the Needs: Author Feedback label Nov 19, 2018

natalieethell requested a review from a team November 26, 2018 21:37

lorejoh12 approved these changes Nov 26, 2018

View reviewed changes

lorejoh12 merged commit 6ec50ae into microsoft:master Nov 26, 2018

lorejoh12 mentioned this pull request Feb 1, 2019

Calendar - "Go to Today" button doesn't do anything when on current month #6873

Closed

microsoft locked as resolved and limited conversation to collaborators Aug 30, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Correct calendar high contrast #6938

Correct calendar high contrast #6938

jhellesen commented Nov 1, 2018 •

edited by fabricteam

Loading

natalieethell commented Nov 6, 2018

jhellesen commented Nov 6, 2018

natalieethell commented Nov 6, 2018

jhellesen commented Nov 6, 2018 •

edited

Loading

lorejoh12 Nov 7, 2018

lorejoh12 Nov 7, 2018

natalieethell commented Nov 7, 2018

aneeshack4 commented Nov 19, 2018

lorejoh12 commented Nov 26, 2018

natalieethell commented Nov 26, 2018

msft-github-bot commented Nov 29, 2018

Correct calendar high contrast #6938

Correct calendar high contrast #6938

Conversation

jhellesen commented Nov 1, 2018 • edited by fabricteam Loading

Pull request checklist

Focus areas to test

Microsoft Reviewers: Open in CodeFlow

natalieethell commented Nov 6, 2018

jhellesen commented Nov 6, 2018

natalieethell commented Nov 6, 2018

jhellesen commented Nov 6, 2018 • edited Loading

lorejoh12 Nov 7, 2018

Choose a reason for hiding this comment

lorejoh12 Nov 7, 2018

Choose a reason for hiding this comment

natalieethell commented Nov 7, 2018

aneeshack4 commented Nov 19, 2018

lorejoh12 commented Nov 26, 2018

natalieethell commented Nov 26, 2018

msft-github-bot commented Nov 29, 2018

jhellesen commented Nov 1, 2018 •

edited by fabricteam

Loading

jhellesen commented Nov 6, 2018 •

edited

Loading