From 6865b74cf1ca78ec158798876b48021d06517c91 Mon Sep 17 00:00:00 2001 From: Boris Goldowsky Date: Tue, 10 Sep 2024 16:28:11 -0400 Subject: [PATCH 1/2] Adjust tile size when legend changes. Restyle 'vertical' layout --- src/plugins/bar-graph/bar-graph-tile.tsx | 56 ++++++++++++++---- src/plugins/bar-graph/bar-graph.scss | 52 ++++++++++++---- src/plugins/bar-graph/legend-area.tsx | 75 +++++++++++++----------- src/plugins/graph/graph-registration.ts | 2 +- 4 files changed, 125 insertions(+), 60 deletions(-) diff --git a/src/plugins/bar-graph/bar-graph-tile.tsx b/src/plugins/bar-graph/bar-graph-tile.tsx index ac095ac6bd..05ceed2236 100644 --- a/src/plugins/bar-graph/bar-graph-tile.tsx +++ b/src/plugins/bar-graph/bar-graph-tile.tsx @@ -1,35 +1,67 @@ -import React from "react"; +import React, { useRef } from "react"; import classNames from "classnames"; import { observer } from "mobx-react"; import { useResizeDetector } from "react-resize-detector"; +import { ChartArea } from "./chart-area"; +import { LegendArea } from "./legend-area"; import { BasicEditableTileTitle } from "../../components/tiles/basic-editable-tile-title"; import { ITileProps } from "../../components/tiles/tile-component"; -import { ChartArea } from "./chart-area"; import { BarGraphModelContext } from "./bar-graph-content-context"; import { isBarGraphModel } from "./bar-graph-content"; import { TileToolbar } from "../../components/toolbar/tile-toolbar"; -import { LegendArea } from "./legend-area"; import "./bar-graph.scss"; import "./bar-graph-toolbar"; const legendWidth = 190; -const legendHeight = 190; // FIXME export const BarGraphComponent: React.FC = observer((props: ITileProps) => { - - const { model, readOnly } = props; + const { model, readOnly, onRequestRowHeight } = props; const content = isBarGraphModel(model.content) ? model.content : null; - const {height: containerHeight, width: containerWidth, ref} = useResizeDetector(); + const requestedHeight = useRef(undefined); + + const onResize = (width: number|undefined, height: number|undefined) => { + let desiredTileHeight; + if (height) { + if (legendBelow) { + const desiredLegendHeight = height; + desiredTileHeight = 300 + desiredLegendHeight; + } else { + const desiredLegendHeight = Math.max(height, 260); // Leave room for at least 5 rows per spec + desiredTileHeight = desiredLegendHeight + 66; + } + if (requestedHeight.current !== desiredTileHeight) { + requestedHeight.current = desiredTileHeight; + onRequestRowHeight(model.id, desiredTileHeight); + } + } + }; + + // We use two resize detectors to track the size of the container and the size of the legend area + const { height: containerHeight, width: containerWidth, ref: containerRef } = useResizeDetector(); + + const { height: legendHeight, ref: legendRef } = useResizeDetector({ + refreshMode: 'debounce', + refreshRate: 500, + skipOnMount: false, + onResize + }); + let svgWidth = 10, svgHeight = 10; + // Legend is on the right if the width is >= 450px, otherwise below const legendBelow = containerWidth && containerWidth < 450; if (containerWidth && containerHeight) { - // Legend is on the right if the width is >= 450px - svgWidth = legendBelow ? containerWidth : containerWidth-legendWidth; - svgHeight = legendBelow ? containerHeight-legendHeight : containerHeight; + if (legendBelow) { + const vertPadding = 18; + svgWidth = containerWidth; + svgHeight = containerHeight - vertPadding - (legendHeight || 0); + } else { + svgWidth = containerWidth - legendWidth; + svgHeight = containerHeight; + } } return ( @@ -37,12 +69,12 @@ export const BarGraphComponent: React.FC = observer((props: ITilePro
- +
); diff --git a/src/plugins/bar-graph/bar-graph.scss b/src/plugins/bar-graph/bar-graph.scss index 3dd8597421..862b76a1c5 100644 --- a/src/plugins/bar-graph/bar-graph.scss +++ b/src/plugins/bar-graph/bar-graph.scss @@ -9,13 +9,7 @@ align-items: center; overflow: auto; - &.vertical { - flex-direction: column; - } - svg.bar-graph-svg { - // width: 100%; - // height: 100%; .visx-bar-group .visx-bar { stroke: black; @@ -104,12 +98,46 @@ } } - &.vertical div.bar-graph-legend { - width: 100%; - height: 186px; - border-top: 1.5px solid #0592af; - border-left: none; - padding: 8px 0 0 0; + // Overrides for vertical (legend underneath) layout + &.vertical { + flex-direction: column; + + div.bar-graph-legend { + width: 100%; + height: auto; + border-top: 1.5px solid #0592af; + border-left: none; + padding: 8px 0 0 0; + + .dataset-header { + margin-left: 5px; + align-items: center; + + .dataset-label-text, .dataset-name { + display: inline-block; + margin-right: 5px; + } + } + + .sort-by { + margin: 0 0 10px 20px; + + button.chakra-menu__menu-button { + width: auto; + margin: 5px 0 0 5px; + } + } + + .secondary-values { + display: flex; + flex-wrap: wrap; + margin-left: 8px; + + .color-button { + margin: 0 7px 0 7px; + } + } + } } button.chakra-menu__menu-button { diff --git a/src/plugins/bar-graph/legend-area.tsx b/src/plugins/bar-graph/legend-area.tsx index 6feb327200..bd68d9eaf2 100644 --- a/src/plugins/bar-graph/legend-area.tsx +++ b/src/plugins/bar-graph/legend-area.tsx @@ -7,8 +7,11 @@ import { LegendSecondaryRow } from './legend-secondary-row'; import RemoveDataIcon from "../../assets/remove-data-icon.svg"; import DropdownCaretIcon from "../../assets/dropdown-caret.svg"; +interface IProps { + legendRef: React.RefObject; +} -export const LegendArea = observer(function LegendArea () { +export const LegendArea = observer(function LegendArea ({legendRef}: IProps) { const model = useBarGraphModelContext(); function unlinkDataset() { @@ -38,44 +41,46 @@ export const LegendArea = observer(function LegendArea () { return (
-
-
- - - +
+
+
+ + + +
+
+ Data from: + {model.dataSet.name} +
-
- Data from: - {model.dataSet.name} -
-
-
-
- Sort by: +
+ + Sort by: + + + + + {currentLabel} + + + + + + setSecondaryAttribute(undefined)}>None + {availableAttributes.map((a) => ( + setSecondaryAttribute(a.id)}>{a.name} + ))} + + +
- - - - {currentLabel} - - - - - - setSecondaryAttribute(undefined)}>None - {availableAttributes.map((a) => ( - setSecondaryAttribute(a.id)}>{a.name} - ))} - - - -
-
- {currentSecondary - ? secondaryKeys.map((key) => ) - : } +
+ {currentSecondary + ? secondaryKeys.map((key) => ) + : } +
); diff --git a/src/plugins/graph/graph-registration.ts b/src/plugins/graph/graph-registration.ts index 7d050940b6..b69a54df04 100644 --- a/src/plugins/graph/graph-registration.ts +++ b/src/plugins/graph/graph-registration.ts @@ -5,10 +5,10 @@ import { GraphWrapperComponent } from "./components/graph-wrapper-component"; import { createGraphModel, GraphModel } from "./models/graph-model"; import { updateGraphContentWithNewSharedModelIds, updateGraphObjectWithNewSharedModelIds } from "./utilities/graph-utils"; + import { AppConfigModelType } from "../../models/stores/app-config-model"; import Icon from "./assets/graph-icon.svg"; import HeaderIcon from "./assets/graph-tile-id.svg"; -import { AppConfigModelType } from "../../models/stores/app-config-model"; function graphAllowsMultipleDataSets(appConfig: AppConfigModelType) { return !!appConfig.getSetting("defaultSeriesLegend", "graph"); From d8acf33d02f022a5b1b47e29a6c112d8585aadd1 Mon Sep 17 00:00:00 2001 From: Boris Goldowsky Date: Wed, 11 Sep 2024 08:29:54 -0400 Subject: [PATCH 2/2] Add test for vertical layout --- .../tile_tests/bar_graph_tile_spec.js | 29 +++++++++++++------ cypress/support/elements/tile/BarGraphTile.js | 4 +++ 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/cypress/e2e/functional/tile_tests/bar_graph_tile_spec.js b/cypress/e2e/functional/tile_tests/bar_graph_tile_spec.js index 329a3b9a4f..829558e4ae 100644 --- a/cypress/e2e/functional/tile_tests/bar_graph_tile_spec.js +++ b/cypress/e2e/functional/tile_tests/bar_graph_tile_spec.js @@ -73,6 +73,15 @@ context('Bar Graph Tile', function () { it('Can link data ', function () { beforeTest(); + clueCanvas.addTile('bargraph'); + barGraph.getTiles().click(); + barGraph.getYAxisLabel().should('have.text', 'Counts'); + barGraph.getXAxisPulldown().should('have.text', 'Categories'); + barGraph.getYAxisTickLabel().should('not.exist'); + barGraph.getXAxisTickLabel().should('not.exist'); + barGraph.getLegendArea().should('not.exist'); + barGraph.getBar().should('not.exist'); + // Table dataset for testing: // 4 instances of X / Y / Z // 2 instances of XX / Y / Z @@ -88,16 +97,8 @@ context('Bar Graph Tile', function () { ['X', 'Y', 'Z'], ]); - clueCanvas.addTile('bargraph'); - barGraph.getTiles().click(); - barGraph.getYAxisLabel().should('have.text', 'Counts'); - barGraph.getXAxisPulldown().should('have.text', 'Categories'); - barGraph.getYAxisTickLabel().should('not.exist'); - barGraph.getXAxisTickLabel().should('not.exist'); - barGraph.getLegendArea().should('not.exist'); - barGraph.getBar().should('not.exist'); - cy.log('Link bar graph'); + barGraph.getTile().click(); clueCanvas.clickToolbarButton('bargraph', 'link-tile'); cy.get('select').select('Table Data 1'); cy.get('.modal-button').contains("Graph It!").click(); @@ -111,6 +112,16 @@ context('Bar Graph Tile', function () { barGraph.getSortByMenuButton().should('have.text', 'None'); barGraph.getSecondaryValueName().should('have.length', 1).and('have.text', 'x'); + cy.log('Legend should move to bottom when tile is narrow'); + barGraph.getTileContent().should('have.class', 'horizontal').and('not.have.class', 'vertical'); + clueCanvas.addTileByDrag('table', 'right'); + clueCanvas.addTileByDrag('table', 'right'); + barGraph.getTileContent().should('have.class', 'vertical').and('not.have.class', 'horizontal'); + clueCanvas.getUndoTool().click(); // undo add table + clueCanvas.getUndoTool().click(); // undo add table + tableTile.getTableTile().should('have.length', 1); + barGraph.getTileContent().should('have.class', 'horizontal').and('not.have.class', 'vertical'); + cy.log('Change Sort By'); barGraph.getSortByMenuButton().click(); barGraph.getChakraMenuItem().should('have.length', 3); diff --git a/cypress/support/elements/tile/BarGraphTile.js b/cypress/support/elements/tile/BarGraphTile.js index 6eeea90682..5fe641923f 100644 --- a/cypress/support/elements/tile/BarGraphTile.js +++ b/cypress/support/elements/tile/BarGraphTile.js @@ -12,6 +12,10 @@ class BarGraphTile { return this.getTile(tileIndex, workspaceClass).find(`.editable-tile-title-text`); } + getTileContent(tileIndex = 0, workspaceClass) { + return this.getTile(tileIndex, workspaceClass).find(`[data-testid="bar-graph-content"]`); + } + getChakraMenuItem(tileIndex = 0, workspaceClass) { return cy.get(`body .chakra-portal button`).filter(':visible'); }