-
-
Notifications
You must be signed in to change notification settings - Fork 25
Add a modal Dialog component #245
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
Conversation
|
Hi @CyberDex please take a look at my PR. I am still working on it, but do you have any suggestions already? |
…ve it a scrollSpeed param
…e other components
|
Hi @Zyie @GoodBoyDigital @CyberDex please review my PR |
|
Hi @CyberDex I have improved the PR:
Below are screenshots demoing this - the Simple Alert story has no animations and the Confirm Dialog story uses custom FancyButtons with hover/press animations:
I don't see any color issues in the Storybook on my end. Could you please confirm if the colors still look weird to you? |
|
I have pushed a few fixes. And there are a few issues to look into:
My main concern is to unify this component. The whole idea of this library is to have all components as universal as possible, so that text can be passed as text (an instance will be created under the hood), same as any text instance can be passed. Let's make sure it works the same for this component. Maybe we could make it as a combination of 2 instances of the Like the outer list can have a background and a Backdrop, it can have a list of instances to be used as content (title, text), this way we could easily configure all margins and paddings of the elements. And for the buttons, it would be a horizontal list to align buttons, that in their own order can be passed as instances of either Button or FancyButton. Sorry for such a deep review, just want to make sure all components follow the same idea and patterns. WDYT @afarber ? |
|
Hi @CyberDex I have addressed all your comments, please review my recent commits. Specifically I have removed the redundand solid colors dialog stories, but please keep the both letter stories for me. I need them for the word game that I am porting from jQuery UI to PixiUi, this is my motivation for the PR. |
8a066b9 to
e3224ad
Compare
65ded03 to
4c7dc2d
Compare
|
I have made some changes. @afarber also had to update your implementation of letter stories, as they were not adaptive, and will merge once you give me a green light. If you are not happy with what I have done, we can talk about it and update |
|
Your changes look great. Yes, please merge and consider a release (new package version) |
|
@afarber I removed an automatic dialog closing on any button click. Let's keep it manual for the developer; this will give a bit more control over the dialog behavior. |



Add a modal Dialog component to PixiUI for asking questions and presenting simple choices:
onSelectsignal with button index and text when user makes a selectionGetViewSettingscloseOnBackdropClickto dismiss on a scrim tap