chore: Fix size behaviour improvements (#47)

* chore: Fix size behaviour improvements

* Update chart-core-fit-size.test.tsx

* fix regressions

* use default height

Author: pan-kot

pages/common/styles.module.scss

@@ -15,6 +15,7 @@
   border: 1px solid cs.$color-border-status-info;
   border-radius: 4px;
   position: relative;
+  overflow: hidden;
 }
 
 .chart-frame-annotation {

pages/common/templates.tsx

@@ -264,7 +264,7 @@ const ChartFrame = forwardRef(
     ref: React.Ref<HTMLDivElement>,
   ) => {
     return (
-      <div ref={ref} className={styles["chart-frame"]} style={{ height, width, overflow: "hidden" }}>
+      <div ref={ref} className={styles["chart-frame"]} style={{ height, width }}>
         {children}
         <div className={styles["chart-frame-annotation"]}>{annotation}</div>
       </div>

src/core/__tests__/chart-core-fit-size.test.tsx

@@ -45,7 +45,8 @@ describe("CoreChart: fit-size", () => {
   test("uses explicit chart height", () => {
     const { rerender } = renderChart({ highcharts });
 
-    expect(HighchartsReact).toHaveBeenCalledWith(chartOptionsWithHeight(undefined), expect.anything());
+    // Default height is used when explicit height is not set.
+    expect(HighchartsReact).toHaveBeenCalledWith(chartOptionsWithHeight(400), expect.anything());
 
     rerender({ highcharts, options: { chart: { height: "20rem" } } });
 
@@ -110,20 +111,27 @@ describe("CoreChart: fit-size", () => {
       const offset = verticalAxisTitlePlacement === "top" ? 28 : 0;
       const { rerender } = renderChart({ highcharts, fitHeight: true, verticalAxisTitlePlacement });
 
+      // Uses default min height of 200px.
       await waitFor(() => {
-        expect(HighchartsReact).toHaveBeenCalledWith(chartOptionsWithHeight(80 - offset), expect.anything());
+        expect(HighchartsReact).toHaveBeenCalledWith(chartOptionsWithHeight(200 - offset), expect.anything());
       });
 
-      rerender({ highcharts, fitHeight: true, chartHeight: 300, verticalAxisTitlePlacement });
-
+      // Uses measured height when it is bigger than explicitly provided min height.
+      rerender({ highcharts, fitHeight: true, chartMinHeight: 50, verticalAxisTitlePlacement });
       await waitFor(() => {
         expect(HighchartsReact).toHaveBeenCalledWith(chartOptionsWithHeight(80 - offset), expect.anything());
       });
 
-      rerender({ highcharts, fitHeight: true, chartHeight: 300, chartMinHeight: 200, verticalAxisTitlePlacement });
+      // Chart height has no effect on charts with fit-height.
+      rerender({ highcharts, fitHeight: true, chartMinHeight: 50, chartHeight: 300, verticalAxisTitlePlacement });
+      await waitFor(() => {
+        expect(HighchartsReact).toHaveBeenCalledWith(chartOptionsWithHeight(80 - offset), expect.anything());
+      });
 
+      // Taking min height which is above the default min height.
+      rerender({ highcharts, fitHeight: true, chartHeight: 300, chartMinHeight: 250, verticalAxisTitlePlacement });
       await waitFor(() => {
-        expect(HighchartsReact).toHaveBeenCalledWith(chartOptionsWithHeight(200 - offset), expect.anything());
+        expect(HighchartsReact).toHaveBeenCalledWith(chartOptionsWithHeight(250 - offset), expect.anything());
       });
     },
   );

src/core/chart-container.tsx

@@ -15,6 +15,9 @@ import testClasses from "./test-classes/styles.css.js";
 // Chart container implements the layout for top-level components, including chart plot, legend, and more.
 // It also implements the height- and width overflow behaviors.
 
+const DEFAULT_CHART_HEIGHT = 400;
+const DEFAULT_CHART_MIN_HEIGHT = 200;
+
 interface ChartContainerProps {
   // The header, footer, vertical axis title, and legend are rendered as is, and we measure the height of these components
   // to compute the available height for the chart plot when fitHeight=true. When there is not enough vertical space, the
@@ -51,17 +54,16 @@ export function ChartContainer({
   chartMinHeight,
   chartMinWidth,
 }: ChartContainerProps) {
+  const { refs, measures } = useContainerQueries();
+
   // The vertical axis title is rendered above the chart, and is technically not a part of the chart plot.
   // However, we want to include it to the chart's height computations as it does belong to the chart logically.
   // We do so by taking the title's constant height into account, when "top" axis placement is chosen.
   const verticalTitleOffset = Styles.verticalAxisTitleBlockSize + Styles.verticalAxisTitleMargin;
   const heightOffset = verticalAxisTitlePlacement === "top" ? verticalTitleOffset : 0;
-  const withMinHeight = (height?: number) =>
-    height === undefined ? chartMinHeight : Math.max(chartMinHeight ?? 0, height) - heightOffset;
-
-  const { refs, measures } = useContainerQueries();
+  const withMinHeight = (height: number) => Math.max(chartMinHeight ?? DEFAULT_CHART_MIN_HEIGHT, height) - heightOffset;
   const measuredChartHeight = withMinHeight(measures.chart - measures.header - measures.footer);
-  const effectiveChartHeight = fitHeight ? measuredChartHeight : withMinHeight(chartHeight);
+  const effectiveChartHeight = fitHeight ? measuredChartHeight : withMinHeight(chartHeight ?? DEFAULT_CHART_HEIGHT);
   return (
     <div
       ref={refs.chart}

src/core/styles.scss

@@ -89,7 +89,12 @@ $side-legend-min-inline-size: max(20%, 150px);
   gap: cs.$space-static-xxs;
 }
 
+.chart-container-root {
+  block-size: 100%;
+}
+
 .chart-container-fit-height {
+  overflow-x: auto;
   position: absolute;
   inset: 0;
 }