Skip to content

Commit

Permalink
fix(frontend): comparison diff ui fixes (#627)
Browse files Browse the repository at this point in the history
* fix(frontend): comparison diff ui fixes

* bring back that table | both | flamegraph in toolbar
* reduce legend size for smaller screens
  • Loading branch information
eh-am committed Dec 20, 2021
1 parent 007c54f commit 202835b
Show file tree
Hide file tree
Showing 8 changed files with 61 additions and 4 deletions.
Binary file modified cypress/snapshots/e2e.ts/e2e-comparison-diff-flamegraph.snap.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified cypress/snapshots/e2e.ts/e2e-comparison-flamegraph-left.snap.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified cypress/snapshots/lang-smoke.ts/comparison-go-1.snap.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified cypress/snapshots/lang-smoke.ts/comparison-go-2.snap.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
6 changes: 5 additions & 1 deletion webapp/javascript/components/ComparisonDiffApp.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,11 @@ function ComparisonDiffApp(props) {
viewSide="both"
/>
<Box>
<FlameGraphRenderer viewType="diff" flamebearer={diff.flamebearer}>
<FlameGraphRenderer
display="both"
viewType="diff"
flamebearer={diff.flamebearer}
>
<div className="diff-instructions-wrapper">
<div className="diff-instructions-wrapper-side">
<InstructionText viewType="diff" viewSide="left" />
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
import React from 'react';
import useResizeObserver from '@react-hook/resize-observer';
import { colorFromPercentage } from './color';
import styles from './DiffLegend.module.css';

export default function DiffLegend() {
const values = [100, 80, 60, 40, 20, 10, 0, -10, -20, -40, -60, -80, -100];
const legendRef = React.useRef();
const showMode = useSizeMode(legendRef);
const values = decideLegend(showMode);

return (
<div
className={`row ${styles['flamegraph-legend']}`}
data-testid="flamegraph-legend"
ref={legendRef}
>
<div className={styles['flamegraph-legend-list']}>
{values.map((v) => (
Expand All @@ -27,3 +31,53 @@ export default function DiffLegend() {
</div>
);
}

function decideLegend(showMode: ReturnType<typeof useSizeMode>) {
switch (showMode) {
case 'large': {
return [100, 80, 60, 40, 20, 10, 0, -10, -20, -40, -60, -80, -100];
}

case 'small': {
return [100, 40, 20, 0, -20, -40, -100];
}

default:
throw new Error(`Unsupported ${showMode}`);
}
}

/**
* TODO: unify this and toolbar's
* Custom hook that returns the size ('large' | 'small')
* that should be displayed
* based on the toolbar width
*/
// arbitrary value
// as a simple heuristic, try to run the comparison view
// and see when the buttons start to overlap
const WIDTH_THRESHOLD = 600;
const useSizeMode = (target: React.RefObject<HTMLDivElement>) => {
const [size, setSize] = React.useState<'large' | 'small'>('large');

const calcMode = (width: number) => {
if (width < WIDTH_THRESHOLD) {
return 'small';
}
return 'large';
};

React.useLayoutEffect(() => {
if (target.current) {
const { width } = target.current.getBoundingClientRect();

setSize(calcMode(width));
}
}, [target.current]);

useResizeObserver(target, (entry: ResizeObserverEntry) => {
setSize(calcMode(entry.contentRect.width));
});

return size;
};
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,8 @@ THIS SOFTWARE.
.pane-wrapper .flamegraph-pane,
.flamegraph-pane {
flex: 1;
width: 100%;
position: relative;
/* margin: 0 6px; */
width: 50%;
}

/*.flamegraph-pane.vertical-orientation {
Expand Down

0 comments on commit 202835b

Please sign in to comment.