chore: Refactor Select and Combobox to use new styling utilities and tokens #2570
Conversation
Passing run #7017 ↗︎
Details:
Review all test suite changes for PR #2570 ↗︎ |
||||||||||||||||||||
Co-authored-by: Josh <44883293+josh-bagwell@users.noreply.github.com>
Co-authored-by: Josh <44883293+josh-bagwell@users.noreply.github.com>
Co-authored-by: Raisa Primerova <48605821+RayRedGoose@users.noreply.github.com>
Co-authored-by: Josh <44883293+josh-bagwell@users.noreply.github.com>
Co-authored-by: Josh <44883293+josh-bagwell@users.noreply.github.com>
| <Box | ||
| ref={model.state.containerRef} | ||
| {...handleCsProp( | ||
| {}, |
There was a problem hiding this comment.
Is this just empty since no elemProps?
There was a problem hiding this comment.
Correct, the consumer will never pass elemProps here, it's passed to the other element below
There was a problem hiding this comment.
I don't think you need Box here at all. there is no elemProps passed, It can be regular div with mergeProps or stencil
There was a problem hiding this comment.
maybe it's good to consider possibility of full transformation of this component? I don't think this will change a functionality of visual at all
| export const ComboboxMenuItemIcon = createSubcomponent('span')({ | ||
| modelHook: useComboboxModel, | ||
| subComponents: { | ||
| Icon: styled(SystemIcon)({alignSelf: 'start'}), |
There was a problem hiding this comment.
Should we be using styled here? Or are we trying to step away from this?
| {inputStartIcon && model.state.selectedIds.length > 0 && ( | ||
| <InputGroup.InnerStart pointerEvents="none"> | ||
| <InputGroup.InnerStart {...selectIconsStencil()}> |
There was a problem hiding this comment.
Does this need mergeStyles? Or are we good with spreading this here?
There was a problem hiding this comment.
Not here because the consumer will never have access to this element. I call mergeStyles where the consumer can pass elemProps too.
| } as const; | ||
| }); | ||
|
|
||
| const comoboxMenuListStencil = createStencil({ | ||
| base: {}, | ||
| extends: menuListStencil, |
mannycarrera4
added
automerge
and removed
ready for review
Summary
Fixes: #2399
Release Category
Components