Closed kum-deepak closed 2 years ago
@gordonwoodhull, I am merging this as I need it for one of the subsequent changes. Please review, I will create a new PR with your feedback.
Nice changes, really cleans things up to work with width and height this way.
I'm sure you're on it, but please continue to update the codemodes as the API changes.
Thanks for your review! Codemods is configuration-driven, adding width
and height
to the configuration was all that it needed (https://github.com/dc-js/dc-codemods/commit/d5eb6cac4e594ae033719e13414344d4f75468bc).
I am considering removing explicit width and height from examples. Still thinking about that.
Do you mean setting the width and height on the elements using CSS instead? Seems like it still needs to be set somewhere.
Yes, that way the examples will be responsive.
I realize that people pick code from examples (like I did when I started using dc), so, if the examples are as per what we would recommend, it will help users.
Oh, you mean making all of them responsive instead of having separate resizing/
examples? That sounds good. I guess we should keep one or two non-responsive for ppl who still want to do that, but I agree responsive is probably the more popular & impressive case.
This implementation picks from #1853. The key points:
width
andheight
are no longer modified by the charts, so, these have now been moved toconfiguration
.width
(orheight
) is provided that will be used, if not the corresponding dimension will be picked as per the CSS box, subject tominWidth
.configuration
there is a callbackbeforeResize
, which can be used to adjust other dimension-related parameters. For example, theresizing-pie
uses it to adjust the inner radius and theresizing-series
uses it to keep the legends right aligned.Future considerations:
minWidhth
andminHeight
smaller, say 50. The minimum should be controlled by CSS if someone needs it. The library default should only be to protect developers from setting the height to0
without realizing it. If we keep it high, users may get frustrated that their responsive charts are not working.