Add WHCM support for all the form elements by jnurthen · Pull Request #2830 · adobe/react-spectrum

jnurthen · 2022-02-10T02:48:49Z

Closes #2529
Closes #3260

This concludes the high contrast PRs

✅ Pull Request Checklist:

Included link to corresponding React Spectrum GitHub Issue.
Added/updated unit tests and storybook for this change (for new code or code which already has tests).
Filled out test instructions.
Updated documentation (if it already exists for this component).
Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

Turn on WHCM
Run Storybook for the following components:

Date and Time -> Calendar
Date and Time -> Date Range Picker
Checkbox
ColorSlider, ColorThumb, ColorWheel
Picker
Combobox
MenuTrigger
Radio Group
Slider
NumberField
TextField
TextArea
Switch

Screenshots

Calendar

Before

#### After

Date Range

Before

#### After

Checkbox

After (compare to XD design)

Color Components

Before

#### After

Combobox

After (compare to XD)

Switch

After (compare to XD)

NumberField

Before

#### After

Slider

Before

After

Radio Group

Before

#### After

into WHCM-inputs

devongovett · 2022-03-11T23:45:54Z

+     &.is-invalid {
+      .spectrum-Checkbox-box {
+        &:before {
+          border-color: var(--spectrum-checkbox-box-border-color);


This conflicts with var(--spectrum-checkbox-box-border-color-error) above. Is the intended border color Highlight or ButtonText?

It is intended to be ButtonText when unselected and Highlight when selected according to the XD
Checkbox (Windows high contrast mode) UI kit

devongovett · 2022-03-11T23:46:49Z

+    --spectrum-checkbox-text-color-error-down: FieldText;
+    --spectrum-checkbox-text-color-error-hover: FieldText;
+
+     &.is-invalid {


I'd prefer if we can only override variables and not re-define selectors if possible. I think most of this seems unnecessary because the vars are already overridden above?

The variables as defined assume the same border colour for :checked.
I don't think I can do this without modifying

.spectrum-Checkbox.is-invalid { /* Extra-specific selectors added here to handle checked state */ .spectrum-Checkbox-input:checked + .spectrum-Checkbox-box, .spectrum-Checkbox-box { &:before { border-color: var(--spectrum-checkbox-box-border-color-error); } }

in lines 158-165 where the same colour is used for checked and unchecked.
I could add a new variable there and split that selector. Would that be better?

@devongovett is that the direction you would like me to go?

devongovett · 2022-03-12T00:18:52Z

+    }
+
+
+  }


In Firefox, the disabled state looks wrong. It has a white background. Seems ok in edge/chrome though.

Also, the invalid/valid icons in Firefox are still red/green rather than white.

Firefox doesn't support forced-color-adjust:none at the moment. To be honest I don't think we have a hope of supporting FF in high contrast until they do unless I simplify the spectrum defined high contrast behaviour.

I'm ok with either direction but as far as I know the high contrast usage with FF is not very high.

@devongovett are we looking to support FF in WHCM?

I'm happy to follow your lead there. If it's easy to support, then great, but if it's not possible or a lot of additional work for not much benefit than it's fine with me if we don't. We should just note that so that when we test it's understood which browsers we should be testing.

majornista · 2022-06-20T14:08:13Z

GET_BUILD

LFDanLu · 2022-06-20T22:57:45Z

GET_BUILD

adobe-bot · 2022-06-20T23:08:57Z

Build successful! 🎉

LFDanLu · 2022-06-24T20:55:54Z

Are there XDs for Calendar/DateRange WHCM? I've looked on Spectrum/contributions but couldn't find any. If not thats fine, I've been trying to watch out for any style ambiguities

jnurthen · 2022-06-24T21:00:02Z

Are there XDs for Calendar/DateRange WHCM? I've looked on Spectrum/contributions but couldn't find any. If not thats fine, I've been trying to watch out for any style ambiguities

No. I don't believe there are up to date XDs for non high contrast mode for this component either.

LFDanLu · 2022-06-24T21:07:52Z

GET_BUILD

adobe-bot · 2022-06-24T21:20:47Z

Build successful! 🎉

LFDanLu · 2022-06-24T21:36:16Z

    }
  }
 }
+


Noticed the following when testing the Calendar/DateRangePicker WHCM. Lemme know if these are as expected due to color limitations or if they aren't/won't be handled in this PR:

Can't see that the user is currently focused on the datepicker button:

vs

The current date (aka the 24th) styling is overridden when the range includes it

vs

The invalid colors are inconsistent between the date range, the validation icons, and the border/focus ring of the input field. The invalid color in the calendar in particular are hard to see on the black blackground (perhaps a limitation of WHCM theming since there isn't an error color?)

vs

Working on this.

I think this is an acceptable compromise. With limited colours knowing today is optional IMO. The colour in the default scheme doesn't meet WCAG so if we insist knowing today is required then we need to fix outside high contrast mode too.

These variables were introduced after I created this PR. I'm adding these new invalid variables which will look identical to a selection. The error message is the way you tell that there is an error

LFDanLu

Some additional things from testing, I'd be happy to approve and handle these in a follow up if you'd like. Apologies for the much delayed review and thank you for working on this!

In order to reproduce the overlapping gradients for a ColorArea, the container element .spectrum-ColorArea will have a background-image or background-color over which the .spectrum-ColorArea-gradient renders.

into WHCM-inputs

LFDanLu · 2022-07-18T22:04:11Z

GET_BUILD

adobe-bot · 2022-07-18T22:18:50Z

Build successful! 🎉

LFDanLu

LGTM, tested again in Windows Chrome, thanks for the fixes!

addressed

jnurthen and others added 25 commits November 12, 2021 15:55

fix: merged checkbox fixes from spectrum-css for WHCM

5ba857c

fix: WHCM for menu

1c01b22

fix: WHCM fixes for textfield

1cbc0ea

fix: WHCM for switch

3d9b038

fix: WHCM checkbox - more fixes

43b053e

fix: WHCM radioGroup

984793b

fix: WHCM slider

71fbb79

fix: WHCM stepper

1aa3afa

fix: merged checkbox fixes from spectrum-css for WHCM

b0ba6fc

fix: WHCM for menu

ccffaf7

fix: WHCM fixes for textfield

138b2c4

fix: WHCM for switch

72a7654

fix: WHCM checkbox - more fixes

8d74956

fix: WHCM radioGroup

8c79395

fix: WHCM slider

bb33844

fix: WHCM stepper

92b7504

Merge branch 'WHCM-inputs' of https://github.com/jnurthen/react-spectrum

6a16ca6

into WHCM-inputs

Add calendar WHCM

7703542

Add InputGroup WHCM

f45332b

Add Color Controls WHCM

f7cda1f

fix linter

a7f7d39

fix: Picker WHCM

e572c56

fix: add WHCM checkbox story

78b6f4c

Merge branch 'main' into WHCM-inputs

ab8cd95

fix: update WHCM combo/menu/picker

26121dc

jnurthen marked this pull request as ready for review February 17, 2022 23:44

jnurthen requested review from dannify, devongovett and snowystinger as code owners February 17, 2022 23:44

devongovett reviewed Mar 12, 2022

View reviewed changes

majornista self-requested a review June 23, 2022 17:19

majornista previously requested changes Jun 23, 2022

View reviewed changes

Comment thread packages/@adobe/spectrum-css-temp/components/calendar/skin.css

devongovett mentioned this pull request Jun 23, 2022

WHCM / forced-colors support missing in useDatePicker, RangeCalendar #3260

Closed

fix: unavailable today date was invisible

06d95f1

LFDanLu reviewed Jun 24, 2022

View reviewed changes

jnurthen and others added 8 commits June 27, 2022 15:15

fix slider down state

e29d077

Fix quiet combobox

26a3d97

Add invalid colours

6726f83

ColorArea should have forced-color-adjust: none

d90330e

In order to reproduce the overlapping gradients for a ColorArea, the container element .spectrum-ColorArea will have a background-image or background-color over which the .spectrum-ColorArea-gradient renders.

fix: combobox/picker/date/textfield hover/active style

3930fec

Merge branch 'WHCM-inputs' of https://github.com/jnurthen/react-spectrum

de82ec6

into WHCM-inputs

Merge branch 'main' into WHCM-inputs

c0c88ac

Merge branch 'main' into WHCM-inputs

23fe1e0

LFDanLu approved these changes Jul 18, 2022

View reviewed changes

Merge branch 'main' into WHCM-inputs

0ee5bc5

dannify approved these changes Jul 19, 2022

View reviewed changes

dannify merged commit 4ae6f5f into adobe:main Jul 19, 2022

dannify added the needs testing label Jul 24, 2022

devongovett removed the needs testing label Aug 19, 2022

Conversation

jnurthen commented Feb 10, 2022 • edited by ktabors Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Pull Request Checklist:

📝 Test Instructions:

Screenshots

Calendar

Before

Date Range

Before

Checkbox

After (compare to XD design)

Color Components

Before

Combobox

After (compare to XD)

Switch

After (compare to XD)

NumberField

Before

Slider

Before

After

Radio Group

Before

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

devongovett Apr 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

majornista commented Jun 20, 2022

Uh oh!

LFDanLu commented Jun 20, 2022

Uh oh!

adobe-bot commented Jun 20, 2022

Uh oh!

Uh oh!

LFDanLu commented Jun 24, 2022

Uh oh!

jnurthen commented Jun 24, 2022

Uh oh!

LFDanLu commented Jun 24, 2022

Uh oh!

adobe-bot commented Jun 24, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LFDanLu left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jnurthen commented Feb 10, 2022 •

edited by ktabors

Loading

devongovett Apr 22, 2022 •

edited

Loading

LFDanLu left a comment •

edited

Loading