Fix chart width on resize and cleanup related code

Calculate AbstractOutputComponent's main area width based on props instead of client width that isn't updated yet during a resize. Ensure minimum of 0 for tree width and chart width. Remove unnecessary paddingRight in ResponsiveGridLayout's style. Add marginRight to TimeAxisComponent and TimeNavigatorComponent to account for scroll bar padding. Remove arbitrary initial width in TraceContextComponent constructor. Make handleWidth, yAxisWidth and sashWidth optional in OutputComponentStyle. Move default handle width constant to AbstractOutputComponent. Add getter for handle width. Move default Y-axis width and sash width constants to AbstractTreeOutputComponent. Add getters for Y-axis width and sash width. Rename sash offset to chart offset. Make chart offset include the handle width, Y-axis width and sash width, which makes these widths irrelevant to components that don't need them. Rename widthWPBugWorkaround to outputWidth. This needs to be stored in props because the ResponsiveGridLayout overrides the output component's style width before render. Calculate TimeNavigatorComponent and TimeAxisComponent width and pass it directly in their props so that they do not need to know about handle width, sash width and chart offset. Reduce TimeNavigatorProps and TimeAxisProps's style to only the necessary attributes. Signed-off-by: Patrick Tasse <patrick.tasse@ericsson.com>
eclipse-cdt-cloud · Feb 9, 2022 · 5dc3985 · 5dc3985
1 parent 6d0d1b8
commit 5dc3985
Show file tree

Hide file tree

Showing 12 changed files with 67 additions and 76 deletions.
diff --git a/packages/react-components/src/components/abstract-output-component.tsx b/packages/react-components/src/components/abstract-output-component.tsx
@@ -25,10 +25,7 @@ export interface AbstractOutputProps {
     markerCategories: string[] | undefined;
     markerSetId: string;
     style: OutputComponentStyle;
-    // WidthProvider (react-grid-layout version 0.16.7) has a bug.
-    // Workaround for it needs width to be explicitly passed
-    // https://github.com/STRML/react-grid-layout/issues/961
-    widthWPBugWorkaround: number;
+    outputWidth: number;
     backgroundTheme: string;
     onOutputRemove: (outputId: string) => void;
     // TODO Not sure
@@ -40,7 +37,7 @@ export interface AbstractOutputProps {
     onMouseDown?: VoidFunction;
     onTouchStart?: VoidFunction;
     onTouchEnd?: VoidFunction;
-    setSashOffset?: (sashOffset: number) => void;
+    setChartOffset?: (chartOffset: number) => void;
 }
 
 export interface AbstractOutputState {
@@ -50,6 +47,8 @@ export interface AbstractOutputState {
 
 export abstract class AbstractOutputComponent<P extends AbstractOutputProps, S extends AbstractOutputState> extends React.Component<P, S> {
 
+    private readonly DEFAULT_HANDLE_WIDTH = 30;
+
     private mainOutputContainer: React.RefObject<HTMLDivElement>;
 
     constructor(props: P) {
@@ -60,9 +59,7 @@ export abstract class AbstractOutputComponent<P extends AbstractOutputProps, S e
     }
 
     render(): JSX.Element {
-        const localStyle = Object.assign({}, this.props.style);
-        localStyle.width = this.props.widthWPBugWorkaround;
-        return <div style={localStyle}
+        return <div style={{ ...this.props.style, width: this.props.outputWidth }}
             id={this.props.traceId + this.props.outputDescriptor.id}
             tabIndex={-1}
             className={'output-container ' + this.props.className}
@@ -75,12 +72,12 @@ export abstract class AbstractOutputComponent<P extends AbstractOutputProps, S e
             <div
                 id={this.props.traceId + this.props.outputDescriptor.id + 'handle'}
                 className='widget-handle'
-                style={{ width: this.props.style.handleWidth, height: this.props.style.height }}
+                style={{ width: this.getHandleWidth(), height: this.props.style.height }}
             >
                 {this.renderTitleBar()}
             </div>
             <div className='main-output-container' ref={this.mainOutputContainer}
-                style={{ width: this.props.widthWPBugWorkaround - this.props.style.handleWidth, height: this.props.style.height }}>
+                style={{ width: this.props.outputWidth - this.getHandleWidth(), height: this.props.style.height }}>
                 {this.renderMainOutputContainer()}
             </div>
             {this.props.children}
@@ -103,15 +100,12 @@ export abstract class AbstractOutputComponent<P extends AbstractOutputProps, S e
         this.props.onOutputRemove(this.props.outputDescriptor.id);
     }
 
-    public getMainAreaWidth(): number {
-        if (this.mainOutputContainer.current) {
-            return this.mainOutputContainer.current.clientWidth;
-        }
-        return this.props.widthWPBugWorkaround - this.props.style.handleWidth;
+    protected getHandleWidth(): number {
+        return this.props.style.handleWidth || this.DEFAULT_HANDLE_WIDTH;
     }
 
-    public getHandleWidth(): number {
-        return this.props.style.handleWidth;
+    public getMainAreaWidth(): number {
+        return this.props.outputWidth - this.getHandleWidth();
     }
 
     private renderMainOutputContainer(): React.ReactNode {

diff --git a/packages/react-components/src/components/abstract-tree-output-component.tsx b/packages/react-components/src/components/abstract-tree-output-component.tsx
@@ -4,6 +4,9 @@ import { ResponseStatus } from 'tsp-typescript-client/lib/models/response/respon
 
 export abstract class AbstractTreeOutputComponent<P extends AbstractOutputProps, S extends AbstractOutputState> extends AbstractOutputComponent<P, S> {
 
+    private readonly DEFAULT_Y_AXIS_WIDTH = 40;
+    private readonly DEFAULT_SASH_WIDTH = 4;
+
     private sashDownX = -1;
     private sashDownOffset = -1;
 
@@ -28,7 +31,7 @@ export abstract class AbstractTreeOutputComponent<P extends AbstractOutputProps,
                 {this.renderYAxis()}
             </div>
             <div className='output-component-sash' onMouseDown={event => this.onSashDown(event)} style={{
-                width: this.props.style.sashWidth, height: this.props.style.height
+                width: this.getSashWidth(), height: this.props.style.height
             }}/>
             <div className='output-component-chart' style={{
                 width: this.getChartWidth(), height: this.props.style.height,
@@ -41,15 +44,16 @@ export abstract class AbstractTreeOutputComponent<P extends AbstractOutputProps,
 
     private onSashDown(event: React.MouseEvent<HTMLDivElement, MouseEvent>): void {
         this.sashDownX = event.clientX;
-        this.sashDownOffset = this.props.style.sashOffset;
+        this.sashDownOffset = this.props.style.chartOffset;
         window.addEventListener('mousemove', this.onSashMove);
         window.addEventListener('mouseup', this.onSashUp);
 }
 
     private onSashMove(ev: MouseEvent): void {
-        if (this.sashDownX !== -1 && this.props?.setSashOffset) {
-            const sashOffset = Math.max(this.props.style.yAxisWidth, this.sashDownOffset + (ev.clientX - this.sashDownX));
-            this.props.setSashOffset(sashOffset);
+        if (this.sashDownX !== -1 && this.props?.setChartOffset) {
+            const chartOffset = Math.max(this.sashDownOffset + (ev.clientX - this.sashDownX),
+                this.getHandleWidth() + this.getYAxisWidth() + this.getSashWidth());
+            this.props.setChartOffset(chartOffset);
             ev.preventDefault();
         }
     }
@@ -88,13 +92,21 @@ export abstract class AbstractTreeOutputComponent<P extends AbstractOutputProps,
 
     }
 
+    protected getYAxisWidth(): number {
+        return this.props.style.yAxisWidth || this.DEFAULT_Y_AXIS_WIDTH;
+    }
+
+    protected getSashWidth(): number {
+        return this.props.style.sashWidth || this.DEFAULT_SASH_WIDTH;
+    }
+
     public getTreeWidth(): number {
         // Make tree thinner when chart has a y-axis
-        const yAxisBuffer = this.props.outputDescriptor.type === 'TREE_TIME_XY' ? this.props.style.yAxisWidth: 0;
-        return Math.min(this.getMainAreaWidth(), this.props.style.sashOffset - yAxisBuffer);
+        const yAxisWidth = this.props.outputDescriptor.type === 'TREE_TIME_XY' ? this.getYAxisWidth(): 0;
+        return Math.max(0, this.props.style.chartOffset - this.getHandleWidth() - yAxisWidth - this.getSashWidth());
     }
 
     public getChartWidth(): number {
-        return Math.max(0, this.getMainAreaWidth() - this.props.style.sashOffset - this.props.style.sashWidth);
+        return Math.max(0, this.props.outputWidth - this.props.style.chartOffset);
     }
 }
diff --git a/packages/react-components/src/components/datatree-output-component.tsx b/packages/react-components/src/components/datatree-output-component.tsx
@@ -103,7 +103,7 @@ export class DataTreeOutputComponent extends AbstractOutputComponent<AbstractOut
         return <React.Fragment>
             {this.state.outputStatus === ResponseStatus.COMPLETED ?
                 <div ref={this.treeRef} className='output-component-tree disable-select'
-                    style={{ height: this.props.style.height, width: this.props.widthWPBugWorkaround }}
+                    style={{ height: this.props.style.height, width: this.props.outputWidth }}
                 >
                     {this.renderTree()}
                 </div> :

diff --git a/packages/react-components/src/components/null-output-component.tsx b/packages/react-components/src/components/null-output-component.tsx
@@ -13,7 +13,7 @@ export class NullOutputComponent extends AbstractOutputComponent<NullOutputProps
     }
 
     renderMainArea(): React.ReactNode {
-        const treeWidth = Math.min(this.getMainAreaWidth(), this.props.style.sashOffset + this.props.style.sashWidth);
+        const treeWidth = Math.min(this.getMainAreaWidth(), this.props.style.chartOffset - this.getHandleWidth());
         const chartWidth = this.getMainAreaWidth() - treeWidth;
         return <React.Fragment>
             <div className='output-component-tree disable-select'

diff --git a/packages/react-components/src/components/table-output-component.tsx b/packages/react-components/src/components/table-output-component.tsx
@@ -101,7 +101,7 @@ export class TableOutputComponent extends AbstractOutputComponent<TableOutputPro
             tabIndex={-1}
             onFocus={event=>this.checkFocus(event)}
             className={this.props.backgroundTheme === 'light' ? 'ag-theme-balham' : 'ag-theme-balham-dark'}
-            style={{ height: this.props.style.height, width: this.props.widthWPBugWorkaround }}>
+            style={{ height: this.props.style.height, width: this.props.outputWidth }}>
             <AgGridReact
                 columnDefs={this.columnArray}
                 rowModelType='infinite'

diff --git a/packages/react-components/src/components/trace-context-component.tsx b/packages/react-components/src/components/trace-context-component.tsx
@@ -63,10 +63,7 @@ export class TraceContextComponent extends React.Component<TraceContextProps, Tr
     private readonly DEFAULT_COMPONENT_ROWHEIGHT: number = 20;
     private readonly DEFAULT_COMPONENT_LEFT: number = 0;
     private readonly SCROLLBAR_PADDING: number = 12;
-    private readonly HANDLE_WIDTH = 30;
-    private readonly Y_AXIS_WIDTH: number = 40;
-    private readonly DEFAULT_SASH_OFFSET = 200;
-    private readonly SASH_WIDTH = 4;
+    private readonly DEFAULT_CHART_OFFSET = 200;
 
     private unitController: TimeGraphUnitController;
     private tooltipComponent: React.RefObject<TooltipComponent>;
@@ -106,11 +103,8 @@ export class TraceContextComponent extends React.Component<TraceContextProps, Tr
             experiment: this.props.experiment,
             traceIndexing: ((this.props.experiment.indexingStatus === this.INDEXING_RUNNING_STATUS) || (this.props.experiment.indexingStatus === this.INDEXING_CLOSED_STATUS)),
             style: {
-                width: this.HANDLE_WIDTH + this.DEFAULT_SASH_OFFSET + this.SASH_WIDTH,
-                handleWidth: this.HANDLE_WIDTH,
-                sashOffset: this.DEFAULT_SASH_OFFSET,
-                sashWidth: this.SASH_WIDTH,
-                yAxisWidth: this.Y_AXIS_WIDTH,
+                width: 0, // width will be set by resize handler
+                chartOffset: this.DEFAULT_CHART_OFFSET,
                 componentLeft: this.DEFAULT_COMPONENT_LEFT,
                 height: this.DEFAULT_COMPONENT_HEIGHT,
                 rowHeight: this.DEFAULT_COMPONENT_ROWHEIGHT,
@@ -139,7 +133,7 @@ export class TraceContextComponent extends React.Component<TraceContextProps, Tr
         this.tooltipXYComponent = React.createRef();
         this.traceContextContainer = React.createRef();
         this.onResize = this.onResize.bind(this);
-        this.setSashOffset = this.setSashOffset.bind(this);
+        this.setChartOffset = this.setChartOffset.bind(this);
         this.props.addResizeHandler(this.onResize);
         this.initialize();
         this.subscribeToEvents();
@@ -267,8 +261,8 @@ export class TraceContextComponent extends React.Component<TraceContextProps, Tr
         this.widgetResizeHandlers.forEach(h => h());
     }
 
-    private setSashOffset(sashOffset: number) {
-        this.setState(prevState => ({ style: { ...prevState.style, sashOffset: sashOffset } }));
+    private setChartOffset(chartOffset: number) {
+        this.setState(prevState => ({ style: { ...prevState.style, chartOffset: chartOffset } }));
         this.widgetResizeHandlers.forEach(h => h());
     }
 
@@ -327,10 +321,11 @@ export class TraceContextComponent extends React.Component<TraceContextProps, Tr
         const layouts = this.generateGridLayout();
         const outputs = this.props.outputs;
         const showTimeScale = outputs.filter(output => output.type === 'TIME_GRAPH' || output.type === 'TREE_TIME_XY').length > 0;
+        const chartWidth = Math.max(0, this.state.style.width - this.state.style.chartOffset);
         return <React.Fragment>
             {showTimeScale &&
-                <div style={{ marginLeft: this.state.style.handleWidth + this.state.style.sashOffset + this.state.style.sashWidth }}>
-                    <TimeAxisComponent unitController={this.unitController} style={this.state.style}
+                <div style={{ marginLeft: this.state.style.chartOffset, marginRight: this.SCROLLBAR_PADDING }}>
+                    <TimeAxisComponent unitController={this.unitController} style={{ ...this.state.style, width: chartWidth }}
                         addWidgetResizeHandler={this.addWidgetResizeHandler} removeWidgetResizeHandler={this.removeWidgetResizeHandler} />
                 </div>
             }
@@ -339,8 +334,7 @@ export class TraceContextComponent extends React.Component<TraceContextProps, Tr
                 // https://github.com/STRML/react-grid-layout/issues/299#issuecomment-524959229
             }
             <ResponsiveGridLayout className='outputs-grid-layout' margin={[0, 5]} isResizable={true} isDraggable={true} resizeHandles={['se', 's', 'sw']}
-                layouts={{ lg: layouts }} cols={{ lg: 1 }} breakpoints={{ lg: 1200 }} rowHeight={this.DEFAULT_COMPONENT_ROWHEIGHT} draggableHandle={'.title-bar-label'}
-                style={{ paddingRight: this.SCROLLBAR_PADDING }}>
+                layouts={{ lg: layouts }} cols={{ lg: 1 }} breakpoints={{ lg: 1200 }} rowHeight={this.DEFAULT_COMPONENT_ROWHEIGHT} draggableHandle={'.title-bar-label'}>
                 {outputs.map(output => {
                     const responseType = output.type;
                     const outputProps: AbstractOutputProps = {
@@ -358,9 +352,9 @@ export class TraceContextComponent extends React.Component<TraceContextProps, Tr
                         style: this.state.style,
                         onOutputRemove: this.props.onOutputRemove,
                         unitController: this.unitController,
-                        widthWPBugWorkaround: this.state.style.width,
+                        outputWidth: this.state.style.width,
                         backgroundTheme: this.state.backgroundTheme,
-                        setSashOffset: this.setSashOffset
+                        setChartOffset: this.setChartOffset
                     };
                     switch (responseType) {
                         case 'TIME_GRAPH':
@@ -378,8 +372,8 @@ export class TraceContextComponent extends React.Component<TraceContextProps, Tr
                 })}
             </ResponsiveGridLayout>
             {showTimeScale &&
-                <div style={{ marginLeft: this.state.style.handleWidth + this.state.style.sashOffset + this.state.style.sashWidth }}>
-                    <TimeNavigatorComponent unitController={this.unitController} style={this.state.style}
+                <div style={{ marginLeft: this.state.style.chartOffset, marginRight: this.SCROLLBAR_PADDING }}>
+                    <TimeNavigatorComponent unitController={this.unitController} style={{ ...this.state.style, width: chartWidth }}
                         addWidgetResizeHandler={this.addWidgetResizeHandler} removeWidgetResizeHandler={this.removeWidgetResizeHandler} />
                 </div>
             }

diff --git a/packages/react-components/src/components/utils/__tests__/time-axis-component.test.tsx b/packages/react-components/src/components/utils/__tests__/time-axis-component.test.tsx
@@ -12,10 +12,7 @@ describe('Time axis component', () => {
     const unitController: TimeGraphUnitController = new TimeGraphUnitController(BigInt(10), { start: BigInt(0), end: BigInt(10)});
     const style: OutputComponentStyle = {
       width: 600,
-      handleWidth: 40,
-      sashOffset: 200,
-      sashWidth: 4,
-      yAxisWidth: 30,
+      chartOffset: 200,
       componentLeft: 0,
       height: 100,
       rowHeight: 100,
@@ -33,10 +30,7 @@ describe('Time axis component', () => {
     const unitController: TimeGraphUnitController = new TimeGraphUnitController(BigInt(10), { start: BigInt(0), end: BigInt(10)});
     const style: OutputComponentStyle = {
       width: 600,
-      handleWidth: 40,
-      sashOffset: 200,
-      sashWidth: 4,
-      yAxisWidth: 30,
+      chartOffset: 200,
       componentLeft: 0,
       height: 100,
       rowHeight: 100,

diff --git a/packages/react-components/src/components/utils/__tests__/time-navigator-component.test.tsx b/packages/react-components/src/components/utils/__tests__/time-navigator-component.test.tsx
@@ -11,9 +11,7 @@ describe('Time axis component', () => {
     const unitController: TimeGraphUnitController = new TimeGraphUnitController(BigInt(10), { start: BigInt(0), end: BigInt(10)});
     const style = {
       width: 600,
-      handleWidth: 40,
-      sashOffset: 200,
-      sashWidth: 4,
+      chartOffset: 200,
       naviBackgroundColor: 0xf4f7fb,
       cursorColor: 0x259fd8,
       lineColor: 0x757575
@@ -27,9 +25,7 @@ describe('Time axis component', () => {
     const unitController: TimeGraphUnitController = new TimeGraphUnitController(BigInt(10), { start: BigInt(0), end: BigInt(10)});
     const style = {
       width: 600,
-      handleWidth: 40,
-      sashOffset: 200,
-      sashWidth: 4,
+      chartOffset: 200,
       naviBackgroundColor: 0xf4f7fb,
       cursorColor: 0x259fd8,
       lineColor: 0x757575

diff --git a/packages/react-components/src/components/utils/output-component-style.ts b/packages/react-components/src/components/utils/output-component-style.ts
@@ -1,9 +1,9 @@
 export interface OutputComponentStyle {
     width: number;
-    handleWidth: number;
-    sashOffset: number;
-    sashWidth: number;
-    yAxisWidth: number;
+    chartOffset: number;
+    handleWidth?: number;
+    yAxisWidth?: number;
+    sashWidth?: number;
     componentLeft: number;
     // react-grid-layout - The library used for resizing components
     // inserts new React components during compilation, and the dimensions

diff --git a/packages/react-components/src/components/utils/time-axis-component.tsx b/packages/react-components/src/components/utils/time-axis-component.tsx
@@ -3,11 +3,15 @@ import { TimeGraphAxis } from 'timeline-chart/lib/layer/time-graph-axis';
 import { TimeGraphAxisCursors } from 'timeline-chart/lib/layer/time-graph-axis-cursors';
 import { ReactTimeGraphContainer } from './timegraph-container-component';
 import { TimeGraphUnitController } from 'timeline-chart/lib/time-graph-unit-controller';
-import { OutputComponentStyle } from './output-component-style';
 
 interface TimeAxisProps {
     unitController: TimeGraphUnitController;
-    style: OutputComponentStyle;
+    style: {
+        width: number,
+        chartBackgroundColor: number,
+        cursorColor: number,
+        lineColor: number
+    };
     addWidgetResizeHandler: (handler: () => void) => void;
     removeWidgetResizeHandler: (handler: () => void) => void;
 }
@@ -18,8 +22,8 @@ export class TimeAxisComponent extends React.Component<TimeAxisProps> {
             id='timegraph-axis'
             options={{
                 id: 'timegraph-axis',
+                width: this.props.style.width,
                 height: 30,
-                width: this.props.style.width - this.props.style.handleWidth - this.props.style.sashOffset - this.props.style.sashWidth,
                 backgroundColor: this.props.style.chartBackgroundColor,
                 lineColor: this.props.style.lineColor,
                 classNames: 'horizontal-canvas'

diff --git a/packages/react-components/src/components/utils/time-navigator-component.tsx b/packages/react-components/src/components/utils/time-navigator-component.tsx
@@ -7,9 +7,6 @@ interface TimeNavigatorProps {
     unitController: TimeGraphUnitController;
     style: {
         width: number,
-        handleWidth: number,
-        sashOffset: number,
-        sashWidth: number,
         naviBackgroundColor: number,
         cursorColor: number,
         lineColor: number
@@ -24,9 +21,9 @@ export class TimeNavigatorComponent extends React.Component<TimeNavigatorProps>
         return <ReactTimeGraphContainer
             id='time-navigator'
             options={{
-                width: this.props.style.width - this.props.style.handleWidth - this.props.style.sashOffset - this.props.style.sashWidth,
-                height: 10,
                 id: 'time-navigator',
+                width: this.props.style.width,
+                height: 10,
                 backgroundColor: this.props.style.naviBackgroundColor,
                 classNames: 'horizontal-canvas'
             }}