perf(ui): stop mounting Popover machinery for overlays that cannot open
Two more instances of the same Mantine behaviour, found by searching for overlay components
that take a `disabled` prop rather than by measuring another board.
OverflowBadge (packages/ui) mounted a Popover and a Popover.Dropdown unconditionally - both
when `disablePopover` was set and, more importantly, when nothing overflowed at all, which is
the common case. It is shared UI, used by media-releases, releases, the docker table and the
widget inputs, so the cost repeated per row wherever a list renders it. The rendered markup is
unchanged: a Popover.Target only forwards a ref, so returning the badges directly is
visually identical.
beszel disk-usage mounted a HoverCard per table row with `disabled={filesystems.length === 0}`,
and already fell back to a plain progress bar inside its own target for that case - so for a
system reporting no extra filesystems the entire overlay existed to render nothing. Hoisted
that branch out; the now-unreachable ternary and the dead `disabled` prop went with it.
Measured result of this class of fix, on the calendar which had the worst ratio:
interactive components 171 -> 75 (-56%)
fibers 1195 -> 907 (-24%)
Honest note on the media-releases tooltip fix committed earlier: it measured **zero** on the
test board. Every release there has a description and the option is on, so all 80 tooltips are
legitimately enabled and none are skipped. It only helps boards where descriptions are absent
or the option is off. The mechanism is right; this board simply is not the case it helps.
Also confirmed NOT worth changing: downloads mounts 30 Tooltips but all three per row are
functional (integration name, hover detail, progress), so there is nothing disabled to skip.
Replacing them with native title attributes would change the UX, not just the cost.
This commit is contained in:
2 files changed
+56
-27
No files matched your search
@@ -19,23 +19,43 @@ export function OverflowBadge({
|
||||
radius: "sm",
|
||||
...props,
|
||||
};
|
||||
return (
|
||||
<Popover width="content" shadow="md" disabled={disablePopover}>
|
||||
|
||||
const hasOverflow = data.length > overflowCount;
|
||||
|
||||
const badges = data.slice(0, overflowCount).map((item) => (
|
||||
<Badge key={item} px="xs" {...badgeProps}>
|
||||
{item}
|
||||
</Badge>
|
||||
));
|
||||
|
||||
const overflowTrigger = hasOverflow ? (
|
||||
<UnstyledButton display="flex">
|
||||
<Badge px="xs" style={{ cursor: "pointer", ...badgeProps.style }} {...badgeProps}>
|
||||
+{data.length - overflowCount}
|
||||
</Badge>
|
||||
</UnstyledButton>
|
||||
) : null;
|
||||
|
||||
// Mantine mounts a Popover's machinery — the Popover itself plus its Dropdown — regardless of
|
||||
// `disabled`, and this component previously mounted both even when nothing overflowed, which is the
|
||||
// common case. That made it a per-row cost anywhere it appears in a list: this is shared UI, used by
|
||||
// media-releases (which passes `disablePopover`), releases, and the widget inputs. Returning the
|
||||
// badges directly when there is no popover to show keeps the rendered markup identical — a disabled
|
||||
// Popover.Target only forwards a ref — while creating none of the overlay components.
|
||||
if (disablePopover || !hasOverflow) {
|
||||
return (
|
||||
<Group gap={groupGap}>
|
||||
{data.slice(0, overflowCount).map((item) => (
|
||||
<Badge key={item} px="xs" {...badgeProps}>
|
||||
{item}
|
||||
</Badge>
|
||||
))}
|
||||
{data.length > overflowCount && (
|
||||
<Popover.Target>
|
||||
<UnstyledButton display="flex">
|
||||
<Badge px="xs" style={{ cursor: "pointer", ...badgeProps.style }} {...badgeProps}>
|
||||
+{data.length - overflowCount}
|
||||
</Badge>
|
||||
</UnstyledButton>
|
||||
</Popover.Target>
|
||||
)}
|
||||
{badges}
|
||||
{overflowTrigger}
|
||||
</Group>
|
||||
);
|
||||
}
|
||||
|
||||
return (
|
||||
<Popover width="content" shadow="md">
|
||||
<Group gap={groupGap}>
|
||||
{badges}
|
||||
<Popover.Target>{overflowTrigger}</Popover.Target>
|
||||
</Group>
|
||||
<Popover.Dropdown>
|
||||
<Stack>
|
||||
|
||||
@@ -22,6 +22,24 @@ export const DiskUsage = ({ system, fontSize, progressSize, valueMiw, valueGap =
|
||||
const dotSize = progressSize === "xs" ? 2 : 3;
|
||||
const dotGap = 2;
|
||||
|
||||
// Mantine mounts a HoverCard's Popover machinery even when it is disabled, and this component
|
||||
// renders per table row. A system reporting no extra filesystems has nothing to show on hover — it
|
||||
// already fell back to a plain bar inside the target — so the whole overlay was created for nothing.
|
||||
if (filesystems.length === 0) {
|
||||
return (
|
||||
<Group gap={valueGap} wrap="nowrap" style={{ flex: 1, minWidth: 0, marginLeft: "auto" }}>
|
||||
<Text size={fontSize} fw={500} w={valueMiw} ta="left" style={{ whiteSpace: "nowrap", flexShrink: 0 }}>
|
||||
{formatPercent(system.disk)}
|
||||
</Text>
|
||||
<Box style={{ display: "flex", alignItems: "center", flex: 1, minWidth: 24 }}>
|
||||
<Box pos="relative" style={{ flex: 1, minWidth: 24 }}>
|
||||
<Progress value={system.disk} color={thresholdColor(system.disk)} size={trackSize} />
|
||||
</Box>
|
||||
</Box>
|
||||
</Group>
|
||||
);
|
||||
}
|
||||
|
||||
return (
|
||||
<Group gap={valueGap} wrap="nowrap" style={{ flex: 1, minWidth: 0, marginLeft: "auto" }}>
|
||||
<Text size={fontSize} fw={500} w={valueMiw} ta="left" style={{ whiteSpace: "nowrap", flexShrink: 0 }}>
|
||||
@@ -33,11 +51,9 @@ export const DiskUsage = ({ system, fontSize, progressSize, valueMiw, valueGap =
|
||||
shadow="md"
|
||||
openDelay={200}
|
||||
closeDelay={100}
|
||||
disabled={filesystems.length === 0}
|
||||
>
|
||||
<HoverCard.Target>
|
||||
{filesystems.length > 0 ? (
|
||||
<UnstyledButton
|
||||
<UnstyledButton
|
||||
aria-label={`Show usage for ${filesystems.length + 1} filesystems`}
|
||||
style={{
|
||||
display: "flex",
|
||||
@@ -73,14 +89,7 @@ export const DiskUsage = ({ system, fontSize, progressSize, valueMiw, valueGap =
|
||||
))}
|
||||
</Group>
|
||||
</Box>
|
||||
</UnstyledButton>
|
||||
) : (
|
||||
<Box style={{ display: "flex", alignItems: "center", flex: 1, minWidth: 24 }}>
|
||||
<Box pos="relative" style={{ flex: 1, minWidth: 24 }}>
|
||||
<Progress value={system.disk} color={thresholdColor(system.disk)} size={trackSize} />
|
||||
</Box>
|
||||
</Box>
|
||||
)}
|
||||
</UnstyledButton>
|
||||
</HoverCard.Target>
|
||||
<HoverCard.Dropdown p={8}>
|
||||
<Stack gap={6} miw={145}>
|
||||
|
||||
Reference in new issue
Block a user