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

Using headless UI for dialog and select #254

Open
1 task
lqt93 opened this issue Jul 5, 2021 · 7 comments
Open
1 task

Using headless UI for dialog and select #254

lqt93 opened this issue Jul 5, 2021 · 7 comments
Labels
enhancement New feature or request

Comments

@lqt93
Copy link
Contributor

lqt93 commented Jul 5, 2021

By suggestion from a friend in webuild community, we should use headless UI for Moai's dialog and select.
Todos:

@lqt93 lqt93 added the enhancement New feature or request label Jul 5, 2021
@huyng12
Copy link
Contributor

huyng12 commented Jul 7, 2021

Hi @lqt93 , what is the difference between current implementation and Headless UI? (pros and cons,..)

@lqt93
Copy link
Contributor Author

lqt93 commented Jul 7, 2021

Currently, Moai's dialog has issue with focusing on its content (see #249). It is complicated to handle the focus event in a dialog, not only the container of a dialog can be focused on, it also needs to support developer to choose what to focus on when dialog open. Headless UI is fully supported focus management and other things like accessibility, transitions, etc, which we haven't provided in our current Dialog yet. If we use it, we could focus on the design and apis to bring good experiment for our developers.

@lqt93
Copy link
Contributor Author

lqt93 commented Jul 7, 2021

@thien-do Could you please give more reasons for why we should implement with Headless UI?

@huyng12
Copy link
Contributor

huyng12 commented Jul 7, 2021

Currently, Moai's dialog has issue with focusing on its content (see #249). It is complicated to handle the focus event in a dialog, not only the container of a dialog can be focused on, it also needs to support developer to choose what to focus on when dialog open. Headless UI is fully supported focus management and other things like accessibility, transitions, etc, which we haven't provided in our current Dialog yet. If we use it, we could focus on the design and apis to bring good experiment for our developers.

I see, but I think if we use Headless UI for this component (Modal and Select), we'll have a reason for using another Headless UI components in the future. If we aim to build a home-made UI Kit, I suggest trying to implement/resolve it by themself.

@thien-do
Copy link
Owner

Good point @huyng12 . Moving forward, I think we should prefer headless UI. In fact, one of my big todo is to have a custom Select based on Downshift (the current one is a simple implementation based on the native select tag).

The Dialog is a compromised choice I made previously to quickly have the Dialog (its demand is high, you know). However, now I think we should have one based on Headless UI, since it's just too much work to handle here.

Again, one of Moai's principles is to leverage the industry's best solutions, and not re-inventing them (https://docs.moaijs.com/?path=/docs/intro-principles--page#interoperable)


Now speaking of the implementation, I'm afraid that revamping the current Dialog using Headless UI's Dialog would be too much work and risk. So we have 2 options I think:

  • Create a new Dialog, maybe called it Dialog2 or AdvancedDialog, which should use Headless UI underlying, and should try to mimic the API of Dialog, and we will encourage our users to move to this, and drop Dialog in v2
  • Provide a complete test suit for the current Dialog, and after that we can confidently revamp it using Headless UI

WDYT?

@lqt93
Copy link
Contributor Author

lqt93 commented Jul 11, 2021

Actually, I am lean on the second option, AdvancedDialog or Dialog2 make no sense. We should make least affection on our client if possible.

@thien-do
Copy link
Owner

So... first thing first we should have more test for the current Dialog. It's just too risk to revamp something without test :D

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

No branches or pull requests

3 participants