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(components): add dialog component #68

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

sonnyt
Copy link
Contributor

@sonnyt sonnyt commented May 8, 2024

Started to add a dialog component. This is a still work in progress.

Am I heading the right direction @brankoconjic ? 😃

image

Copy link

changeset-bot bot commented May 8, 2024

⚠️ No Changeset found

Latest commit: 2664d26

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

vercel bot commented May 8, 2024

@sonnyt is attempting to deploy a commit to the Lemon Squeezy Team on Vercel.

A member of the Team first needs to authorize it.

@brankoconjic brankoconjic self-requested a review May 8, 2024 17:21
@brankoconjic
Copy link
Contributor

Hey @sonnyt,

Thank you for your contribution. It seems like you're headed in the right direction! Here are a few comments:

You've used some Tailwind utilities that do not exist. These animation classes are available through the Wedges Tailwind CSS plugin:

  • "wg-fade-in-up"
  • "wg-fade-in-down"
  • "wg-fade-in-left"
  • "wg-fade-in-right"
  • "wg-fade-out"
  • "wg-line-spinner"

We don't provide these: animate-in, animate-out...

Also, text-wedges-gray-900, you probably meant text-wg-gray-900.

Close button

I'd use the Wedges Button component for the Dialog Close component to maintain consistency with the design.

DialogContent.displayName = "DialogContent";
DialogOverlay.displayName = "DialogOverlay";

const Dialog = Object.assign(DialogPrimitive.Root, {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea behind Wedges components is to provide a versatile yet easy to use component for common use cases, equipped with the most useful props. Also, there is an option to enhance and personalize these components through the composition of additional components.

For instance, the Dialog component can be used as follows:

<Dialog title="Title" isOpen={true}>
  Content
</Dialog>

For more advanced use cases, the component can be composed with other components:

<Dialog.Root>
  <Dialog.Title>Title</Dialog.Title>
  <Dialog.Content>
    Content
  </Dialog.Content>
 ...
</Dialog.Root>

This flexibility allows for both easy implementation in straightforward scenarios and customizable complexity when needed.
```suggestion
const Dialog = Object.assign(DialogPrimitive.Root, {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect!

I will work on it next week :)

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

Successfully merging this pull request may close these issues.

None yet

2 participants