-
Notifications
You must be signed in to change notification settings - Fork 55
Conversation
3d1f5a1
to
d4e3a24
Compare
9a1d24f
to
bc81b50
Compare
Perf comparison
Potential regressions comparing to master
Perf tests with no regressions
Generated by 🚫 dangerJS |
const { disabled } = this.props | ||
const checked = !this.state.checked | ||
const handleClick = (e: React.MouseEvent | React.KeyboardEvent) => { | ||
const checked = !state.checked |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: move this line in the condition below
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done 👍
|
||
return ( | ||
<ElementType {...getA11Props('root', { className: classes.root, ...unhandledProps })}> | ||
{isSvgIcon && callable(maybeIcon.icon)({ classes, rtl: context.rtl, props })} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should restrict the props sent here too. Not as part of this PR, though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree, let's do it separately. I am even not sure that should pass there something
const { classes, styles: resolvedStyles } = useStyles(Label.displayName, { | ||
className: Label.className, | ||
mapPropsToStyles: () => ({ | ||
hasActionableIcon: _.has(icon, 'onClick'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
@@ -124,7 +124,7 @@ exports[`felaRenderer marginLeft is rendered into marginLeft due to RTL with \`n | |||
|
|||
|
|||
<div className=ui-provider__box dir=rtl> | |||
<span className=ui-text a dir=auto> | |||
<span dir=auto className=ui-text a> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder why these are changed :\
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small issue, I fixed order of props in Text
and reverted this change 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It scares me that Checkbox
does not render but no UTs fail 😟
…ithub.com/stardust-ui/react into chore/move-more-components � Conflicts: � CHANGELOG.md
/** Alternative text. */ | ||
alt?: string | ||
'aria-label'?: string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
...commonPropTypes.createCommon({ | ||
children: false, | ||
content: false, | ||
const iconElement = Icon.create(icon, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No getA11Props
?
const { className, color, icon, size, state, design, styles, variables } = props | ||
const { classes, styles: resolvedStyles } = useStyles(Status.displayName, { | ||
className: Status.className, | ||
mapPropsToStyles: () => ({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's update the statusStyles
with the new typings
@@ -106,15 +106,15 @@ export type TeamsThemeStylesProps = { | |||
Reaction?: ReactionProps | |||
ReactionGroup?: ReactionGroupProps | |||
Segment?: SegmentProps | |||
Slider?: SliderProps | |||
Slider?: SliderStylesProps |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's update Status
too.
…ithub.com/stardust-ui/react into chore/move-more-components
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please update #2269 after merging
BREAKING CHANGES
Only specific props are now passed to component's style functions.
Checkbox
checked
disabled
labelPosition
toggle
Icon
bordered
circular
color
disabled
name
outline
rotate
size
xSpacing
isFontIcon
isSvgIcon
Label
hasActionableIcon
hasImage
circular
color
imagePosition
Slider
fluid
vertical
disabled
Status
color
size
state
Text
atMention
color
important
timestamp
truncated
disabled
error
success
temporary
align
weight
size