chore: Refactor SearchForm to use our styling API #3303
Conversation
Workday/canvas-kit
|
||||||||||||||||||||||||||||||||||||||||
| Project |
Workday/canvas-kit
|
| Branch Review |
mc-search-from-refactor
|
| Run status |
|
| Run duration | 02m 55s |
| Commit |
|
| Committer | Manuel Carrera |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
2
|
Pending
|
21
|
Skipped
|
0
|
Passing
|
936
|
| View all changes introduced in this branch ↗︎ | |
UI Coverage
21.19%
|
|
|---|---|
|
Untested elements
|
1538
|
|
Tested elements
|
411
|
Accessibility
99.29%
|
|
|---|---|
|
Failed rules
|
6 critical
5 serious
0 moderate
2 minor
|
|
Failed elements
|
97
|
…rera4/canvas-kit into mc-search-from-refactor
Co-authored-by: Raisa Primerova <[email protected]>
…rera4/canvas-kit into mc-search-from-refactor
| @@ -15,8 +15,8 @@ export interface SearchThemeAttributes { | |||
| colorFocus?: string; | |||
| placeholderColor?: string; | |||
| placeholderColorFocus?: string; | |||
| boxShadow?: string | string[]; | |||
| boxShadowFocus?: string | string[]; | |||
There was a problem hiding this comment.
This technically a breaking change, but in most cases you would just pass a string so I think it's ok. This wouldn't work with the stencil if it accepted an array i think.
There was a problem hiding this comment.
This is okay. The original code was matching the Emotion types where an array of strings meant the boxShadow property would be listed multiple times. This is done for browser fallbacks when a property isn't understood by the parser. It is extremely rare to do this now with our drop in IE11 support.
boxShadow: ['first', 'second']
// CSS
box-shadow: first;
box-shadow: second;
| expect(chroma(style.color || '').hex()).toBe(searchThemes[theme].color); | ||
| expect(style.boxShadow).toBe(searchThemes[theme].boxShadow); | ||
| }); | ||
|
|
There was a problem hiding this comment.
These test where visual, so i deleted them an instead added visual regression tests.
modules/labs-react/search-form/stories/examples/CustomTheme.tsx
Outdated
Show resolved
Hide resolved
| @@ -118,173 +117,318 @@ function getInputColors(theme: SearchThemeAttributes, isFocused?: boolean) { | |||
| }; | |||
| } | |||
|
|
|||
| const formCollapsedBackground = colors.frenchVanilla100; | |||
| const formCollapsedBackground = '#fff'; | |||
There was a problem hiding this comment.
This is used for chroma and I can't have a css variable. I'm not actually styling the button, it's just used in getIconButtonType to determine if it's inverse or not.
| grow: { | ||
| true: { | ||
| maxWidth: '100%', | ||
| [`[data-part="search-form-field"], [data-part="search-form-input"]`]: { |
There was a problem hiding this comment.
I had to do this because parts aren't part of the compound modifiers.
| console.log('theme.backgroundFocus', theme.backgroundFocus); | ||
| console.log(isFocused); | ||
| console.log('theme background', theme.background); |
There was a problem hiding this comment.
Are we removing these?
modules/labs-react/search-form/stories/examples/CustomTheme.tsx
Outdated
Show resolved
Hide resolved
…rera4/canvas-kit into mc-search-from-refactor
modules/labs-react/search-form/stories/examples/CustomTheme.tsx
Outdated
Show resolved
Hide resolved
Co-authored-by: Josh <[email protected]>
…rera4/canvas-kit into mc-search-from-refactor
| background: colors.cinnamon600, | ||
| backgroundFocus: colors.frenchVanilla100, | ||
| placeholderColor: colors.frenchVanilla100, | ||
| placeholderColorFocus: colors.blackPepper400, |
There was a problem hiding this comment.
Should colors used here?
There was a problem hiding this comment.
ah no, I'll add some random hex stuff
Summary
Fixes: #3272
Release Category
Components
Release Note
SearchThemeAttributestype has been updated. BothboxShadowandboxShadowFocusnow only accept astringinstead ofstring | string[].SearchThemeenum type has been updated to have string valueslight,darkandtransparent. This should not affect the way you use the type unless you're passing anumberof0,1or2to thesearchThemeprop.Checklist
ready for reviewhas been added to PRFor the Reviewer
Where Should the Reviewer Start?
Areas for Feedback? (optional)
Testing Manually
Screenshots or GIFs (if applicable)
Thank You Gif (optional)