Skip to content
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

Closed
wants to merge 6 commits into from

Conversation

jainpawan21
Copy link
Member

@jainpawan21 jainpawan21 commented Jun 24, 2024

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

@jainpawan21 jainpawan21 changed the title feat(web, novui): add modal recipe initial feat(web, novui): add modal component novui Jun 24, 2024
Copy link

netlify bot commented Jun 24, 2024

Deploy Preview for novu-design failed. Why did it fail? →

Name Link
🔨 Latest commit a72c162
🔍 Latest deploy log https://app.netlify.com/sites/novu-design/deploys/66843d4cc4e8fa00083688f0

Copy link

netlify bot commented Jun 24, 2024

Deploy Preview for dev-web-novu ready!

Name Link
🔨 Latest commit a72c162
🔍 Latest deploy log https://app.netlify.com/sites/dev-web-novu/deploys/66843d4c72316000083fdf81
😎 Deploy Preview https://deploy-preview-5808--dev-web-novu.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Contributor

@antonjoel82 antonjoel82 left a 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.

libs/novui/src/components/modal/Modal.tsx Outdated Show resolved Hide resolved
libs/novui/src/components/modal/Modal.tsx Show resolved Hide resolved
libs/novui/src/recipes/modal.recipe.ts Outdated Show resolved Hide resolved
libs/novui/src/recipes/modal.recipe.ts Show resolved Hide resolved
libs/novui/src/tokens/semanticSizes.tokens.ts Show resolved Hide resolved
libs/novui/src/recipes/modal.recipe.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@antonjoel82 antonjoel82 left a 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.

Screenshot 2024-07-03 at 7 07 43 AM Screenshot 2024-07-03 at 7 08 13 AM

Some things to note or think about:

  1. Title text doesn't match
  2. Padding is too small
  3. 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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants