fix(#2949): TableView/ListView/DnD Windows High Contrast issues by majornista · Pull Request #3827 · adobe/react-spectrum

majornista · 2022-12-08T22:40:10Z

Relates to #2949

✅ Pull Request Checklist:

Included link to corresponding React Spectrum GitHub Issue 2949.
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:

Using Google Chrome or Microsoft Edge, open https://reactspectrum.blob.core.windows.net/reactspectrum/5d6f3c03ccdf2462277f32e53624cceea767e751/storybook/iframe.html?providerSwitcher-express=false&providerSwitcher-toastPosition=bottom&strict=false&providerSwitcher-locale=&providerSwitcher-theme=&providerSwitcher-scale=&args=&id=tableview--allowsresizing-uncontrolled-sortable-columns&viewMode=story
Open Developer Tools, and enable forced-colors: active emulation under the Customize and control DevTools menu button (looks like "⋮" in the upper right corner of DevTools panel toolbar) > More tools... > Rendering > Emulate CSS media feature forced-colors > forced-colors: active
Use keyboard to navigate rows and cells in the TableView. Confirm that a visual indication of focus is present in forced-colors mode.
Select one or more rows. Confirm that it is possible to distinguish between selected and unselected rows, as well as the focused state of selected rows.
Resize columns to verify that the resize column indicator is visible

Also test ListView stories, including drag and drop: https://reactspectrum.blob.core.windows.net/reactspectrum/e23100ba3eff9c6206f91b525d06f5620eedb5d4/storybook/index.html?path=/story/listview--default&providerSwitcher-express=false&providerSwitcher-toastPosition=bottom

And Drag and Drop stories:
https://reactspectrum.blob.core.windows.net/reactspectrum/5d6f3c03ccdf2462277f32e53624cceea767e751/storybook/index.html?path=/story/drag-and-drop--default&providerSwitcher-express=false&providerSwitcher-toastPosition=bottom

Checkbox WHCM story:
https://reactspectrum.blob.core.windows.net/reactspectrum/5d6f3c03ccdf2462277f32e53624cceea767e751/storybook/index.html?path=/story/checkbox--whcm&providerSwitcher-express=false&providerSwitcher-toastPosition=bottom

Switch WHCM story:
https://reactspectrum.blob.core.windows.net/reactspectrum/5d6f3c03ccdf2462277f32e53624cceea767e751/storybook/index.html?path=/story/switch--whcm-test&providerSwitcher-express=false&providerSwitcher-toastPosition=bottom

🧢 Your Project:

Adobe/Accessibility

…le-whcm

snowystinger

From a glance, we're going to need to set variables for these to cut down on the size of the changes
See what I did to the resizer changes

majornista · 2022-12-15T19:33:14Z

From a glance, we're going to need to set variables for these to cut down on the size of the changes See what I did to the resizer changes

I wonder if the CSS-specificity hack might work to fix things like .spectrum-Checkbox, which has variables defined that are not getting consistently applied.

snowystinger · 2022-12-15T19:58:24Z

Can you say more about "not consistently getting applied"?

majornista · 2022-12-15T20:48:25Z

Can you say more about "not consistently getting applied"?

The updates to Checkbox come from me noticing that in forced-colors: active mode the Spectrum color was still being used for the border/background style of checkboxes, rather than the system color overrides:

react-spectrum/packages/@adobe/spectrum-css-temp/components/checkbox/skin.css

Lines 212 to 241 in 6d71896

    
           forced-color-adjust: none; 
        
           --spectrum-checkbox-box-border-color: ButtonText; 
        
           --spectrum-checkbox-box-border-color-error: Highlight; 
        
           --spectrum-checkbox-box-border-color-error-down: Highlight; 
        
           --spectrum-checkbox-box-border-color-error-hover: Highlight; 
        
           --spectrum-checkbox-box-border-color-key-focus: Highlight; 
        
           --spectrum-checkbox-checkmark-color: HighlightText; 
        
           --spectrum-checkbox-emphasized-box-background-color: ButtonFace; 
        
           --spectrum-checkbox-emphasized-box-background-color-disabled: ButtonFace; 
        
           --spectrum-checkbox-emphasized-box-border-color: ButtonText; 
        
           --spectrum-checkbox-emphasized-box-border-color-disabled: GrayText; 
        
           --spectrum-checkbox-emphasized-box-border-color-down: Highlight; 
        
           --spectrum-checkbox-emphasized-box-border-color-hover: Highlight; 
        
           --spectrum-checkbox-emphasized-box-border-color-selected: Highlight; 
        
           --spectrum-checkbox-emphasized-box-border-color-selected-down: Highlight; 
        
           --spectrum-checkbox-emphasized-box-border-color-selected-hover: Highlight; 
        
           --spectrum-checkbox-emphasized-box-border-color-selected-key-focus: Highlight; 
        
           --spectrum-checkbox-emphasized-text-color: FieldText; 
        
           --spectrum-checkbox-emphasized-text-color-down: FieldText; 
        
           --spectrum-checkbox-emphasized-text-color-hover: FieldText; 
        
           --spectrum-checkbox-emphasized-text-color-key-focus: FieldText; 
        
           --spectrum-checkbox-focus-ring-color-key-focus: ButtonText; 
        
           --spectrum-checkbox-quiet-box-border-color-selected: Highlight; 
        
           --spectrum-checkbox-quiet-box-border-color-selected-down: Highlight; 
        
           --spectrum-checkbox-quiet-box-border-color-selected-hover: Highlight; 
        
           --spectrum-checkbox-text-color: FieldText; 
        
           --spectrum-checkbox-text-color-disabled: GrayText; 
        
           --spectrum-checkbox-text-color-error: FieldText; 
        
           --spectrum-checkbox-text-color-error-down: FieldText; 
        
           --spectrum-checkbox-text-color-error-hover: FieldText;

Here's a screen shot showing the colors that are not being overridden, the default checked and indeterminate states, and the unchecked read-only state:

majornista · 2022-12-15T23:11:33Z

@snowystinger and @devongovett WHCM for Checkbox breaks with this: 9d21a67#diff-33c2361c79a3779c4950c8d95cd3af4b4e3a6ad989803452330f0337ffa64d97. The color styles defined in the index.css are taking precedence over the styles defined in the skin.css.

rspbot · 2023-01-30T16:11:12Z

Build successful! 🎉

snowystinger

Ran chromatic, looks like the selection border changed a little bit https://www.chromatic.com/test?appId=5f0dd5ad2b5fc10022a2e320&id=63d7fc08564648dc28ef74e4
I'm looking into the why of it right now.

Looks like the dropshadow ::after is now --spectrum-table-row-border-color-selected <=== var(--spectrum-legacy-color-blue-500,var(--spectrum-global-color-blue-500))

and it used to be --spectrum-global-color-blue-500
I'm not sure why the legacy one is the lead choice and not the fallback, I see you followed some other examples. @devongovett maybe I'm just not understanding xvar, but shouldn't these all be switched?

Edit: I see why the legacy was preferred there, see description of #3857

snowystinger

got some additional info regarding xvar
it is also used to not collapse variables, much like we have done by placing the variable declarations in the component's top level class.
I'm not sure we should move to using xvar, but just providing some more context

rspbot · 2023-01-30T19:09:54Z

Build successful! 🎉

rspbot · 2023-01-30T19:14:26Z

Build successful! 🎉

rspbot · 2023-01-30T21:10:24Z

Build successful! 🎉

rspbot · 2023-01-30T23:07:02Z

Build successful! 🎉

reidbarber

LGTM

rspbot · 2023-01-31T16:37:11Z

Build successful! 🎉

rspbot · 2023-01-31T16:37:14Z

## API Changes

unknown top level export { type: 'identifier', name: 'Column' }
unknown top level export { type: 'identifier', name: 'Column' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }

snowystinger · 2023-01-31T16:51:28Z

Seems like it's hanging, going to close and reopen

snowystinger · 2023-01-31T16:54:19Z

One more try

Michael Jordan added 2 commits December 8, 2022 17:38

fix(#2949): TableView Windows High Contrast issues

5dc3054

Merge branch 'main' into table-whcm

5df49b3

majornista requested review from devongovett, jnurthen and snowystinger and removed request for devongovett, jnurthen and snowystinger December 9, 2022 15:06

Michael Jordan added 2 commits December 9, 2022 17:20

fix(#2949): ListView Windows High Contrast issues

68e44b0

Merge branch 'table-whcm' of github.com:adobe/react-spectrum into tab…

afa196c

…le-whcm

majornista changed the title ~~fix(#2949): TableView Windows High Contrast issues~~ fix(#2949): TableView/ListView Windows High Contrast issues Dec 12, 2022

fix(#2949): Drag and Drop Windows High Contrast issues

21b1b1c

majornista changed the title ~~fix(#2949): TableView/ListView Windows High Contrast issues~~ fix(#2949): TableView/ListView/DnD Windows High Contrast issues Dec 13, 2022

Merge branch 'main' into table-whcm

b0c5fe9

snowystinger reviewed Dec 15, 2022

View reviewed changes

Merge branch 'main' into table-whcm

78707d3

Merge branch 'main' into table-whcm

655575e

fix(#3672): refine Checkbox WHCM/forced-colors styles

044cb2b

majornista requested a review from snowystinger December 16, 2022 18:30

majornista added Checkbox TableView accessibility ListView labels Dec 16, 2022

This comment was marked as outdated.

Sign in to view

Merge branch 'main' into table-whcm

5d6f3c0

snowystinger reviewed Jan 30, 2023

View reviewed changes

fix border focus colors

b32554b

snowystinger reviewed Jan 30, 2023

View reviewed changes

Merge branch 'main' into table-whcm

e0b58b2

Merge branch 'main' into table-whcm

19d2641

snowystinger approved these changes Jan 30, 2023

View reviewed changes

majornista requested a review from reidbarber January 30, 2023 22:59

Merge branch 'main' into table-whcm

c78018f

reidbarber approved these changes Jan 31, 2023

View reviewed changes

Merge branch 'main' into table-whcm

fb65c3f

snowystinger closed this Jan 31, 2023

snowystinger reopened this Jan 31, 2023

snowystinger closed this Jan 31, 2023

snowystinger reopened this Jan 31, 2023

reidbarber merged commit fb3e5e2 into main Jan 31, 2023

reidbarber deleted the table-whcm branch January 31, 2023 17:01

jnurthen mentioned this pull request Feb 7, 2023

HCM: checkbox colors incorrect for selected styles #4029

Closed

majornista mentioned this pull request Feb 17, 2023

ProgressCircle: Fill color that indicates progress is not visible in HCM for all variants #4069

Closed

yihuiliao mentioned this pull request Nov 10, 2023

Fix selected checkbox fill color when checkbox group is disabled #5407

Merged

5 tasks

Conversation

majornista commented Dec 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Pull Request Checklist:

📝 Test Instructions:

🧢 Your Project:

Uh oh!

snowystinger 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!

majornista commented Dec 15, 2022

Uh oh!

snowystinger commented Dec 15, 2022

Uh oh!

majornista commented Dec 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

majornista commented Dec 15, 2022

Uh oh!

This comment was marked as outdated.

rspbot commented Jan 30, 2023

Uh oh!

snowystinger 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!

snowystinger left a comment

Choose a reason for hiding this comment

Uh oh!

rspbot commented Jan 30, 2023

Uh oh!

rspbot commented Jan 30, 2023

Uh oh!

rspbot commented Jan 30, 2023

Uh oh!

rspbot commented Jan 30, 2023

Uh oh!

reidbarber left a comment

Choose a reason for hiding this comment

Uh oh!

rspbot commented Jan 31, 2023

Uh oh!

rspbot commented Jan 31, 2023

Uh oh!

snowystinger commented Jan 31, 2023

Uh oh!

snowystinger commented Jan 31, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

majornista commented Dec 8, 2022 •

edited

Loading

snowystinger left a comment •

edited

Loading

majornista commented Dec 15, 2022 •

edited

Loading

snowystinger left a comment •

edited

Loading