Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve DevTools Profiler commit-selector UX #20943

Merged
merged 2 commits into from
Mar 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import AutoSizer from 'react-virtualized-auto-sizer';
import {FixedSizeList} from 'react-window';
import SnapshotCommitListItem from './SnapshotCommitListItem';
import {minBarWidth} from './constants';
import {formatDuration, formatTime} from './utils';
import Tooltip from './Tooltip';

import styles from './SnapshotCommitList.css';

Expand All @@ -24,6 +26,7 @@ export type ItemData = {|
selectedCommitIndex: number | null,
selectedFilteredCommitIndex: number | null,
selectCommitIndex: (index: number) => void,
setHoveredCommitIndex: (index: number) => void,
startCommitDrag: (newDragState: DragState) => void,
|};

Expand Down Expand Up @@ -166,6 +169,10 @@ function List({
}
}, [dragState]);

const [hoveredCommitIndex, setHoveredCommitIndex] = useState<number | null>(
null,
);

// Pass required contextual data down to the ListItem renderer.
const itemData = useMemo<ItemData>(
() => ({
Expand All @@ -176,6 +183,7 @@ function List({
selectedCommitIndex,
selectedFilteredCommitIndex,
selectCommitIndex,
setHoveredCommitIndex,
startCommitDrag: setDragState,
}),
[
Expand All @@ -186,22 +194,37 @@ function List({
selectedCommitIndex,
selectedFilteredCommitIndex,
selectCommitIndex,
setHoveredCommitIndex,
],
);

let tooltipLabel = null;
if (hoveredCommitIndex !== null) {
const commitDuration = commitDurations[hoveredCommitIndex];
const commitTime = commitTimes[hoveredCommitIndex];
tooltipLabel = `${formatDuration(commitDuration)}ms at ${formatTime(
commitTime,
)}s`;
Comment on lines +205 to +207
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pan to build on top of this in a future PR that adds the concept of commit times and nested update flags directly to the Profiler UI. Those will be added to this tooltip as well.

}

return (
<div ref={divRef} style={{height, width}}>
<FixedSizeList
className={styles.List}
layout="horizontal"
height={height}
itemCount={filteredCommitIndices.length}
itemData={itemData}
itemSize={itemSize}
ref={(listRef: any) /* Flow bug? */}
width={width}>
{SnapshotCommitListItem}
</FixedSizeList>
</div>
<Tooltip label={tooltipLabel}>
<div
ref={divRef}
style={{height, width}}
onMouseLeave={() => setHoveredCommitIndex(null)}>
<FixedSizeList
className={styles.List}
layout="horizontal"
height={height}
itemCount={filteredCommitIndices.length}
itemData={itemData}
itemSize={itemSize}
ref={(listRef: any) /* Flow bug? */}
width={width}>
{SnapshotCommitListItem}
</FixedSizeList>
</div>
</Tooltip>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,13 @@
display: flex;
align-items: flex-end;
}
.Outer:hover {
background-color: var(--color-background);
}

.Inner {
width: 100%;
min-height: 5px;
min-height: 2px;
background-color: var(--color-commit-did-not-render-fill);
color: var(--color-commit-did-not-render-fill-text);
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ function SnapshotCommitListItem({data: itemData, index, style}: Props) {
maxDuration,
selectedCommitIndex,
selectCommitIndex,
setHoveredCommitIndex,
startCommitDrag,
} = itemData;

Expand All @@ -39,9 +40,21 @@ function SnapshotCommitListItem({data: itemData, index, style}: Props) {
const commitDuration = commitDurations[index];
const commitTime = commitTimes[index];

// Guard against commits with duration 0
const percentage =
// Use natural log for bar height.
// This prevents one (or a few) outliers from squishing the majority of other commits.
// So rather than e.g. _█_ we get something more like e.g. ▄█_
const heightScale =
Math.min(
1,
Math.max(0, Math.log(commitDuration) / Math.log(maxDuration)),
) || 0;

// Use a linear scale for color.
// This gives some visual contrast between cheaper and more expensive commits
// and somewhat compensates for the log scale height.
const colorScale =
Math.min(1, Math.max(0, commitDuration / maxDuration)) || 0;

const isSelected = selectedCommitIndex === index;

// Leave a 1px gap between snapshots
Expand All @@ -62,6 +75,7 @@ function SnapshotCommitListItem({data: itemData, index, style}: Props) {
<div
className={styles.Outer}
onMouseDown={handleMouseDown}
onMouseEnter={() => setHoveredCommitIndex(index)}
style={{
...style,
width,
Expand All @@ -75,9 +89,9 @@ function SnapshotCommitListItem({data: itemData, index, style}: Props) {
<div
className={styles.Inner}
style={{
height: `${Math.round(percentage * 100)}%`,
height: `${Math.round(heightScale * 100)}%`,
backgroundColor:
percentage > 0 ? getGradientColor(percentage) : undefined,
commitDuration > 0 ? getGradientColor(colorScale) : undefined,
}}
/>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
height: 100%;
min-width: 30px;
margin-left: 0.25rem;
overflow: hidden;
overflow: visible;
}
.Commits:focus {
outline: none;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
background-color: var(--color-tooltip-background);
color: var(--color-tooltip-text);
opacity: 1;
white-space: nowrap;
/* Make sure this is above the DevTools, which are above the Overlay */
z-index: 10000002;
}
Expand Down