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: Dialog Component #886

Merged
merged 23 commits into from Jan 5, 2024

Conversation

iamtouha
Copy link
Contributor

@iamtouha iamtouha commented Jan 1, 2024

Description

This PR introduces-

  • Dialog and DialogPanel component
  • Storybook for Dialog
  • Test for Dialog

Dialog API

Name Type Default value Description
open* boolean - Open/Close state
onClose* (val: boolean) => void - Triggers when Dialog open/close state changes
static boolean false Whether the element should ignore the internally managed open/closed state.
unmount boolean true Whether the element should be unmounted or hidden based on the open/closed state.
overlayClassName string - Update Dialog overlay style.

Related issue(s)
refs #830

What kind of change does this PR introduce? (check at least one)

  • Bug fix (non-breaking change which fixes an issue)
  • New Feature (non-breaking change which adds functionality)
  • New Feature (BREAKING CHANGE which adds functionality)
  • Refactor
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

If yes, please describe the impact and migration path for existing applications:

How has this been tested?

  • Unit test written for Dialog

Screenshots (if appropriate):
image

The PR fulfils these requirements:

  • It's submitted to the main branch
  • When resolving a specific issue, it's referenced in the related issue section above
  • My change requires a change to the documentation. (Managed by Tremor Team)
  • I have added tests to cover my changes
  • Check the "Allow edits from maintainers" option while creating your PR.
  • Add refs #XXX or fixes #XXX to the related issue section if your PR refers to or fixes an issue.
  • By contributing to Tremor, you confirm that you have read and agreed to Tremor's CONTRIBUTING.md guideline. You also agree that your contributions will be licensed under the Apache License 2.0 license.

Copy link

vercel bot commented Jan 1, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
tremor-storybook ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 5, 2024 2:19pm

@iamtouha iamtouha changed the title Feature: Dialog Component feat: Dialog Component Jan 1, 2024
@severinlandolt severinlandolt self-assigned this Jan 5, 2024
@severinlandolt severinlandolt merged commit 188449f into tremorlabs:beta-dialog Jan 5, 2024
8 checks passed
Copy link

github-actions bot commented Jan 5, 2024

🎉 This PR is included in version 3.13.0-beta-dialog.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

severinlandolt added a commit that referenced this pull request Jan 7, 2024
* feat: Dialog Component (#886)

* basic dialog component

---------

Co-authored-by: severinlandolt <sev.landolt@gmail.com>
Co-authored-by: christopherkindl <53372002+christopherkindl@users.noreply.github.com>

* Update README.md

* Update dialog

* fix: dialog story

* fix: add max-w default to panel

* fix: classify background as theme token

---------

Co-authored-by: Touha Zohair <39002455+touha98@users.noreply.github.com>
Co-authored-by: christopherkindl <53372002+christopherkindl@users.noreply.github.com>
@severinlandolt
Copy link
Member

severinlandolt commented Jan 9, 2024

Hey @iamtouha, thank you very much for creating this PR. I merged it a few days ago and its almost ready to be included in our next release.

Today I wanted to update Headless UI from 1.7.17 to1.7.18 and in the updated version, there seems to be a type issue. I am having a hard time fixing it, could you take a look at it? I created a branch with the updated tailwind packages and Dialog here:

https://github.com/tremorlabs/tremor/tree/chore/updatetailwindpackagegs

PS: I recently learned, that Headless UI also exports its props, might be useful too:
CleanShot 2024-01-09 at 17 18 21@2x

@iamtouha
Copy link
Contributor Author

iamtouha commented Jan 9, 2024

Hi @severinlandolt I'll take a look at the branch and investigate the type issue. I'll do my best to address it and get things sorted out.

@severinlandolt
Copy link
Member

severinlandolt commented Jan 9, 2024

Hi @severinlandolt I'll take a look at the branch and investigate the type issue. I'll do my best to address it and get things sorted out.

@iamtouha Oh wow, you are quick! Thanks a ton✨
FYI: The error occurs during the build step:

CleanShot 2024-01-09 at 17 56 42@2x

@iamtouha
Copy link
Contributor Author

iamtouha commented Jan 9, 2024

Hi @severinlandolt It seems headlessui introduced a new property for dialog in their latest release. tailwindlabs/headlessui#2709.

the fix is to add role?: "dialog" | "alertdialog"; key in DialogProps

I didn't use the DialogProp type provided by headlessui as typescript would yell at me if I wanted to extend the type. I guess it is due to static and unmount dialog props.

P.S. I have no idea how can i push this change to the branch. Should I create another PR?

image

@severinlandolt
Copy link
Member

severinlandolt commented Jan 9, 2024

@iamtouha Cool! One moment, I will add you as contributor. Makes things a lot easier, have a look at your emails ;) You should now be able to directly commit to this PR 👍

@iamtouha
Copy link
Contributor Author

iamtouha commented Jan 9, 2024

@severinlandolt Cool! thanks!

@iamtouha
Copy link
Contributor Author

iamtouha commented Jan 9, 2024

Hi @severinlandolt, I apologize for any inconvenience or confusion caused by my inexperience. It seems a new PR was created whin i tried to commit to this PR as this is already closed.

@severinlandolt
Copy link
Member

Hey @iamtouha, no worries at all! I will check it right now :)

severinlandolt added a commit that referenced this pull request Jan 11, 2024
* feat: add custom colors for Tracker (#872)

* fix: add CustomColor type

* feat: tickGap (#890)

* fix: remove lib styles (#894)

* feat: Dialog Component (#886)

Co-authored-by: mbauchet <90607026+mbauchet@users.noreply.github.com>
Co-authored-by: Touha Zohair <39002455+touha98@users.noreply.github.com>
Co-authored-by: christopherkindl <53372002+christopherkindl@users.noreply.github.com>
Co-authored-by: severinlandolt <sev.landolt@gmail.com>
@severinlandolt
Copy link
Member

severinlandolt commented Jan 11, 2024

Hi @iamtouha, we just released a new version including your Dialog component! Thank you for your contribution. I was able to release your PR almost unchanged with what you submitted.

If you would like to contribute in the future, we have two issues which we would love to get some help:

@iamtouha
Copy link
Contributor Author

Thanks @severinlandolt. Excited that the Dialog component is part of the new release. It was my pleasure contributing and I'm definitely interested in contributing further. I'll take a look at these issues and see how I can assist.

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

3 participants