Closed renshenhe closed 7 years ago
@renshenhe Thank you for your feedback, I suggest that now do not use Dialog
, need to continue to improve this component, if I set the
padding
properties can be controlled, the effect should be better, I would like to hear your voice.
@renshenhe This project will support css-in-js
and inline-style
both styles in the near future, some of the bugs that are not very serious will resolved after the branch merge, because it will change most of the code.
@myxvisual Sorry I could not respond sooner, been busy.
The ContentDialog
would work fine as a temporary Dialog
component if content
prop accepted reactNode
instead of string or access to it's attributes/styles. I am unsure whether there was intention in making it have a minimum height or accepts a string only so I cannot comment on it's design.
Personally I would prefer a Dialog
component with an empty body that is controlled visibly through props and onBlur
but I have only glanced at the source code of ContentDialog
so my assessment could be lacking.
I am looking forward to the reimplementation using css-in-js
as inline-style
leaves much wanting and a messy browser inspections.
So far the library has been pleasant to work with, though I have many personal preferences when it comes to the current design of the library:
Components are not built with respective html elements e.g. CheckBox
does not use input
therefore it does not have target.value
making it's use a little awkward sometimes.
Preference of passing Event
object rather than value
or checked
for majority of component event handlers. i.e checked => onChangeValue(checked)
is less preferable than event => onChange(event)
Animations(speed control & transition type): Tabs
component for example could have option for animation speed control as animation style. Tabs
current content animation is quite uncomfortable for me seeing it slide right and left every transition instead of sliding relevant to their position in the tabs order. e.g. Sibling to Sibling
No need to take my suggestions, the library is great, but these are just things I found lacking when rewriting my app with react-uwp
.
Thanks for listening!
@renshenhe Thanks for useful advice.
@renshenhe i already update ContentDialog
to Docs, you can try some new API to build your custom UI, the Tabs
is more powerfull then before.
@myxvisual What changed exactly with ContentDialog
? I couldn't find anything significant in the recent commits. The issue I have with ContentDialog
is that content
property has a minimum height which makes contentNode
have a large gap above it. Any reason why content
can't just accept a react node?
@renshenhe hi, now ContentDialog
is support custom padding
props, it could help you solve your requirement?
The ContentDialog Docs
@myxvisual It was never the padding issue, it was the min-height: 160px
on content-dialog-titleWrapper
that meant if contentNode
would always have a 160px top gap if no values were passed to title
or content
. My requirements would simply be removing title
and content
and just having contentNode
so the component would be more customizable.
If ContentDialog
was designed for a specific purpose than that's fine but since the library currently does not have a stand alone Dialog
component which leaves a lot to be desired.
@renshenhe sure, now i know what you want, i will continue to develop the Dialog
component.
@renshenhe If this update has solves your needs, you can close this issue, thanks.
@myxvisual thanks for the implementation, really active commitment! I haven't gotten a chance to dive into the new Dialog component as much of my current usage is ContentDialog
and removing min-heigh
from title-wrapper
at the moment.
I have taken a slight glance at it and seems far more customizable so it looks good!
https://github.com/myxvisual/react-uwp/tree/master/src/Dialog Seems available in the source code but no documentation or usable. Is this a stalled component? Current
ContentDialog
isn't very versatile sincecontent
takes a string and usingcontentNode
withoutcontent
leaves a large gap.