Refactor: Dialog component

hikaya-io / hakawati

A collection of UI components

GNU General Public License v3.0

2 stars 1 forks source link

Refactor: Dialog component #59

Closed ninetteadhikari closed 4 years ago

ninetteadhikari commented 4 years ago

Is your feature request related to a problem? Please describe.

Basic Dialog

Refactor Dialog component to make it reusable. To that end the following needs to be moved from the component to the story to allow for the Dialog to be customized:
- Modal trigger button
- Modal width
- Click handler for the Confirm button
- Label of the Confirm button
Change handleClose to close the Dialog instead of opening another nested Dialog

Form Dialog

Same changes as the Basic dialog
Move form-item and input fields out of the component so that it is not hardcoded. This will allow the component to be used as a shell where any form item and input fields can be entered.

ninetteadhikari commented 4 years ago

hey @hvitis sorry for not being clear in my ticket earlier but as I look at the Dialog component more I realize we can change some of the items to make it more reusable. If it's possible it will be great if you can take a stab at it. Let me know if you have any questions :)

andrewtpham commented 4 years ago

@Tanvir-rahman would you like to take this card on? Reach out to @ninetteadhikari if you have questions

Tanvir-rahman commented 4 years ago

Sure @andrewtpham :)

Tanvir-rahman commented 4 years ago

Hello, @ninetteadhikari i have some queries -> Basic Dialog 1) The button will be removed from the component. And based on dialogVisible prop modal will be visible/invisible right? 2) There is already a click handler for confirm button. Are you requesting something different?

Form Dialog 1) Yeah we can use slots. Need to look further more how we can handle this....

ninetteadhikari commented 4 years ago

Hello, @ninetteadhikari i have some queries -> Basic Dialog

The button will be removed from the component. And based on dialogVisible prop modal will be visible/invisible right?

There is already a click handler for confirm button. Are you requesting something different?

Form Dialog

Yeah we can use slots. Need to look further more how we can handle this....

Hey! Basic dialog

Yes the button would be removed from the component since we want to keep it flexible on what the users clicks on to open the Dialog
I think the clickHandler is currently in the component. I was thinking of exposing it so that we can pass whatever function we want instead of hard coding it. What do you think?

Form dialog

Cool 😎

Tanvir-rahman commented 4 years ago

Hello, @ninetteadhikari. Yeah i saw some implementation. I think we are already exposing it through 'dialogConfirmed', 'dialogFormSubmitted'. Is there something else are you expecting ?

ninetteadhikari commented 4 years ago

Hello, @ninetteadhikari. Yeah i saw some implementation. I think we are already exposing it through 'dialogConfirmed', 'dialogFormSubmitted'. Is there something else are you expecting ?

hey sorry for not responding earlier, yes you are right we don't need the clickHandler then 😊