-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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(web, novui): add modal component novui #5808
Conversation
❌ Deploy Preview for novu-design failed. Why did it fail? →
|
✅ Deploy Preview for dev-web-novu ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
Great progress, Pawan! Could you please add some Storybook stories as we've done with other components? This is the recommended way to develop these in isolation for quick dev and to be able to demo them.
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.
Hey Pawan, thanks for adding the Storybook and other updates!
Based on the Storybook, things don't quite match designs yet. I think we need some more changes here to make this helpful for devs.
Some things to note or think about:
- Title text doesn't match
- Padding is too small
- To provide value, we may want to add some other things based on our sample designs that build behavior in for developers such as:
- a subtitle prop that has opinionated styling within the component
- a way to add buttons (that conforms to designs)
Let me know if you have any questions!
What changed? Why was the change needed?
Add Modal component in novui
Screenshots
Expand for optional sections
Related enterprise PR
Special notes for your reviewer