feat: adds styled icon base component #1791
Conversation
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
| export const StyledBaseIcon = styled(({ children, theme, ...props }) => | ||
| React.cloneElement(Children.only(children), props) |
There was a problem hiding this comment.
props.theme appears to be excluded already. Was it still landing in the DOM?
There was a problem hiding this comment.
Strange, it still shows up for me unless i omit the prop here 🤷🏻
There was a problem hiding this comment.
Oh, I see what's happening. theme is omitted in a normal styled.div (or any tag) call. But because of the cloneElement, we need to manually omit it.
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
| export const StyledBaseIcon = styled(({ children, theme, ...props }) => | ||
| React.cloneElement(Children.only(children), props) |
There was a problem hiding this comment.
[nit] prefer a { cloneElement } import over React.cloneElement.
packages/theming/src/index.ts
Outdated
| @@ -24,6 +24,7 @@ export { useWindow } from './utils/useWindow'; | |||
| export { useText } from './utils/useText'; | |||
| export { default as menuStyles } from './utils/menuStyles'; | |||
| export { focusStyles, SELECTOR_FOCUS_VISIBLE } from './utils/focusStyles'; | |||
| export { StyledBaseIcon } from './styled/StyledBaseIcon'; | |||
There was a problem hiding this comment.
Should this just live in utils? We have other ...Styles utilities in there based heavily on styled-components. I'm thinking we can avoid this one-off.
There was a problem hiding this comment.
Yeah that feels better.
| 'data-garden-id': COMPONENT_ID, | ||
| 'data-garden-version': PACKAGE_VERSION | ||
| })` | ||
| ${props => retrieveComponentStyles(COMPONENT_ID, props)}; | ||
| `; | ||
|
|
||
| StyledBaseIcon.defaultProps = { | ||
| theme: DEFAULT_THEME | ||
| }; |
There was a problem hiding this comment.
Recommend removing from the base.
geotrev
force-pushed
the
george/styled-icon-base
branch
from
12d9ea8
to
af814ff
Compare
Co-authored-by: Jonathan Zempel <jzempel@gmail.com>
geotrev
force-pushed
the
george/styled-icon-base
branch
from
91624d6
to
5693bed
Compare
Description
Adds the base styled icon component to
react-theming, along with updates to two components to demonstrate usage.A follow up PR will update the remaining component instances (13 in total).
Detail
The primary goals of this base component:
cloneElementcall centralizedthemefrom the resulting prop spreadSeparately, using this PR as an opportunity for transient props to prevent further prop-to-DOM leaking in the affected components.
Checklist
👌 design updates will be Garden Designer approved (add the designer as a reviewer)npm start)?bedrock)💂♂️ includes new unit tests. Maintain existing coverage (always >= 96%)♿ tested for WCAG 2.1 AA accessibility compliance