vaadin / vaadin-charts

Vaadin Charts is a feature-rich interactive graph library that answers the data visualization needs of modern web applications
https://vaadin.com/charts
Other
51 stars 24 forks source link

fix: chart reflow if container is flex child #581

Closed sissbruecker closed 3 years ago

sissbruecker commented 3 years ago

Description

Preliminary solution for fixing the regression where setting an explicit width on a chart through the Highcharts API does not work anymore (https://github.com/vaadin/web-components/issues/2600), which was introduced by a recent BFP fix (#580).

The previous fix introduced a CSS rule that applied a width: 100% !important on the inner Highcharts SVG, which breaks charts that have an explicit size set through the Highcharts API. This PR changes the style rule to use max-width: 100%, which still fixes the original issue of the BFP (chart width exceeds container width, if container is a flex child), but is more flexible in terms of sizing.

There is still one scenario not covered by this style rule, which is setting an explicit chart width that is larger than 100% of the container. To fix this I introduced a new attribute has-explicit-width on <vaadin-chart> which gets set / updated whenever an explicit width is set through the Highcharts API. If the attribute is set, then the style rule mentioned above will be disabled to respect the explicit size of the SVG. This solution involves some monkey-patching of Highcharts.Chart - we need to decide if this case is worth doing that, or if we just don't want to cover it.

Still needs tests to verify that explicit sizing works, and that the has-explicit-width attribute logic works.

Fixes https://github.com/vaadin/web-components/issues/2600

Type of change

CLAassistant commented 3 years ago

CLA assistant check
All committers have signed the CLA.

sissbruecker commented 3 years ago

Closing this after internal discussion. We decided that the original issue reported in https://github.com/vaadin/web-components/issues/2600 is expected behaviour due to how flexbox works. Instead we are going to roll back the previous fix.