-
Notifications
You must be signed in to change notification settings - Fork 321
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(Label): celebration animation #2032
Conversation
@@ -62,12 +65,12 @@ const Label: VibeComponent<LabelProps> & { | |||
getStyle(styles, camelCase("kind" + "-" + kind)), | |||
getStyle(styles, camelCase("color" + "-" + color)), | |||
{ | |||
[styles.withAnimation]: !isAnimationDisabled, | |||
[styles.withAnimation]: !isAnimationDisabled && !celebration, |
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 shouldn't work with the regular animation that we have today
type={Text.types.TEXT2} | ||
className={classNames} | ||
color={Text.colors.ON_INVERTED} | ||
data-celebration-text={celebration} |
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.
Need this to style the text, couldn't find a better way
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.
Why not class?
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 can't use a moduled class because I'm targeting the selector from the animation wrapper component
stroke-dashoffset: var(--container-perimeter); | ||
|
||
&.base { | ||
stroke: #ffcc00; |
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.
Need to discuss colors with designers as they are legacy colors, will be replaced with variables
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.
After we will change all the colors let's QA how it looks in all the different products themes :)
|
||
export default LabelCelebrationAnimation; | ||
|
||
function getPath({ |
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.
This is all just math haha
<Text element="span" type={Text.types.TEXT2} className={classNames} color={Text.colors.ON_INVERTED}> | ||
<Text element="span" type={Text.types.TEXT2} color={Text.colors.INHERIT}> | ||
{text} | ||
<LabelCelebrationAnimation active={celebration && kind === "line"}> |
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 shouldn't work in the fill variant
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.
Very very impressive work!!
packages/core/src/components/Label/LabelCelebrationAnimation.tsx
Outdated
Show resolved
Hide resolved
type={Text.types.TEXT2} | ||
className={classNames} | ||
color={Text.colors.ON_INVERTED} | ||
data-celebration-text={celebration} |
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.
Why not class?
To celebrate new feature, outline label can be highlighted by adding celebrate animation. | ||
|
||
<Canvas> | ||
<Story of={LabelStories.Celebration} /> |
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 saw Yossi opened a PR with the deletion of the tag, maybe change it to <Canvas of=
Untitled.mp4
https://monday.monday.com/boards/3532714909/pulses/6162325457