feat: Add inverted variant to Tipseen and Steps #1995
Conversation
There was a problem hiding this comment.
We need to nest this stylesheet sometime, it would be easier to read and understand
| .kindSecondary.colorOnInvertedBackgroundActive, | ||
| .kindSecondary.colorOnInvertedBackground:hover, | ||
| .kindSecondary.colorOnInvertedBackground:focus { | ||
| background: var(--icon-color); |
There was a problem hiding this comment.
I know, it is the same issue that I talked with you about it yesterday, we don't have semantic colors for the inverted colors :(
| @@ -17,7 +19,7 @@ export interface StepsCommandProps extends VibeComponentProps { | |||
| stepsCount: number; | |||
| isIconHidden?: boolean; | |||
| buttonProps?: ButtonProps; | |||
| isOnPrimary?: boolean; | |||
| color?: StepsColor; | |||
There was a problem hiding this comment.
In principle, all these components should not be used by themselves, they are the children of the Steps component where I did not break the api
| @@ -14,3 +14,9 @@ export enum StepsDotAriaCurrent { | |||
| DATE = "date", | |||
| TIME = "time" | |||
| } | |||
|
|
|||
| export enum StepsColor { | |||
There was a problem hiding this comment.
Is it possible that we use TS types here instead of enums? Less for the migration
There was a problem hiding this comment.
Sure! I forgot we talked about it, I'll change it in all the files I touched
| @@ -52,17 +53,17 @@ export const StepsCommand: FC<StepsCommandProps> = ({ | |||
| kind={Button.kinds.TERTIARY} | |||
| onClick={onClick} | |||
| disabled={isDisabled} | |||
| color={buttonBaseColor} | |||
| // @ts-ignore | |||
| color={isOnPrimary ? Button.colors.ON_PRIMARY_COLOR : undefined} | ||
| {...finishButtonProps} | ||
| > | ||
| // @ts-ignore |
There was a problem hiding this comment.
because color is type StepsColor and and ButtonColor
There was a problem hiding this comment.
Do they have the same values? I wonder if we can do something about it
| color={isOnPrimary ? Text.colors.ON_PRIMARY : Text.colors.PRIMARY} | ||
| className={cx(styles.numbers)} | ||
| >{`${activeStepIndex + 1} \\ ${stepsCount}`}</Text> | ||
| // @ts-ignore |
packages/core/src/components/Tipseen/TipseenBasicContent.module.scss
Outdated
Show resolved
Hide resolved
| @@ -1,2 +1,2 @@ | |||
| export { default as video } from "./video.mp4"; | |||
| export { default as picture } from "./picture.png"; | |||
| export { default as picture } from "./picture.svg"; | |||
There was a problem hiding this comment.
Nitpick: background.svg or something else haha
| ### Default | ||
| ### Colors | ||
|
|
||
| Tipseens come in 2 colors: inverted and primary. \ |
There was a problem hiding this comment.
Yes, to break a line
There was a problem hiding this comment.
Oh didn't know that. Double enter also works no?
| > | ||
| <div className="monday-storybook-tipseen_column-item monday-storybook-tipseen_hovered"> | ||
| <Icon icon={Board} iconSize="24" /> | ||
| <span>Sales Pipeline</span> |
There was a problem hiding this comment.
Just wondering, is the span needed?
There was a problem hiding this comment.
Not sure, I copied the example that already existed and deleted things from it, I'll see if I can remove that too
packages/core/src/components/Tipseen/__stories__/tipseen.stories.helpers.js
Show resolved
Hide resolved
https://monday.monday.com/boards/3532714909/pulses/6069570213