-
Notifications
You must be signed in to change notification settings - Fork 322
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(Modal): modal component with basic functionality #2417
feat(Modal): modal component with basic functionality #2417
Conversation
5dc52ba
to
f100fee
Compare
export interface ModalProps extends VibeComponentProps { | ||
show: boolean; | ||
size?: ModalSize; | ||
closeButtonTheme?: ModalTopActionsProps["color"]; |
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.
@shlomitc suggests deciding on a different structure for this, that we would apply on all our components.
Right now the pattern is also "[prefix]Theme" for Tipseen for example. (also for close button theming).
let's discuss if we want to change it.
Should this also include design?
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.
Not sure I understand
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.
we discussed it within the team and decided to leave the theme
pattern
.overlay { | ||
position: fixed; | ||
inset: 0; | ||
background-color: rgba(41, 47, 76, 0.7); |
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.
Is it ok? I think we recently changed it to be more adjustable to the themes, no?
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'll create a separate task for it
export interface ModalProps extends VibeComponentProps { | ||
show: boolean; | ||
size?: ModalSize; | ||
closeButtonTheme?: ModalTopActionsProps["color"]; |
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.
Not sure I understand
ab039e7
to
29b4ff9
Compare
5359a54
into
feat/yossi/new-modal-building-blocks-7359960492
Modal 🌐
This should be a canvas-like component that should only take care of the logic and not what to render inside (except the top actions).
Props:
Usage example:
https://monday.monday.com/boards/3532715121/pulses/7360583166