Skip to content

Commit 84240da

Browse files
ibetitsmikeibetitsmike
authored
fix(site/src/pages/AgentsPage): avoid skills popup flash (coder#25661)
When removing the `/` personal skill trigger, the popover content stayed mounted during its close transition and briefly rendered the empty skills state at the viewport origin. This keeps the menu content mounted for stable Radix positioning, preserves the last open menu state during the close transition, and adds a Storybook regression for the backspace path. > Mux is creating this PR on behalf of Mike.
1 parent 846aac2 commit 84240da

2 files changed

Lines changed: 81 additions & 14 deletions

File tree

‎site/src/pages/AgentsPage/components/ChatMessageInput/ChatMessageInput.stories.tsx‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,13 @@ const expectNoVisibleText = async (text: string) => {
7676
});
7777
};
7878

79+
const expectNoVisibleTextImmediately = (text: string) => {
80+
const matches = within(document.body).queryAllByText(text);
81+
expect(
82+
matches.every((element) => element.getClientRects().length === 0),
83+
).toBe(true);
84+
};
85+
7986
const editorFromCanvas = (canvasElement: HTMLElement) => {
8087
const canvas = within(canvasElement);
8188
return canvas.getByTestId("chat-message-input");
@@ -192,6 +199,18 @@ export const SlashInsideUrlDoesNotOpen: Story = {
192199
},
193200
};
194201

202+
export const BackspaceClosesWithoutEmptyStateFlash: Story = {
203+
play: async ({ canvasElement }) => {
204+
const editor = await typeInEditor(canvasElement, "/");
205+
await findVisibleText("/reviewer");
206+
await userEvent.keyboard("{Backspace}");
207+
208+
expect(editor.textContent).toBe("");
209+
expectNoVisibleTextImmediately("No personal skills found.");
210+
await expectNoVisibleText("/reviewer");
211+
},
212+
};
213+
195214
export const EscapeClosesWithoutReplacing: Story = {
196215
play: async ({ canvasElement }) => {
197216
const editor = await typeInEditor(canvasElement, "/");

‎site/src/pages/AgentsPage/components/ChatMessageInput/PersonalSkillsTriggerMenu.tsx‎

Lines changed: 62 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { useLayoutEffect, useState } from "react";
12
import type * as TypesGen from "#/api/typesGenerated";
23
import {
34
Command,
@@ -35,6 +36,15 @@ type PersonalSkillsTriggerMenuProps = {
3536
onClose: () => void;
3637
};
3738

39+
type PersonalSkillsMenuState = {
40+
anchorRect: CaretAnchorRect;
41+
query: string;
42+
skills: readonly TypesGen.UserSkillMetadata[];
43+
isLoading?: boolean;
44+
isError?: boolean;
45+
selectedIndex: number;
46+
};
47+
3848
export const PersonalSkillsTriggerMenu = ({
3949
open,
4050
anchorRect,
@@ -47,34 +57,72 @@ export const PersonalSkillsTriggerMenu = ({
4757
onSelect,
4858
onClose,
4959
}: PersonalSkillsTriggerMenuProps) => {
60+
const [lastOpenMenuState, setLastOpenMenuState] =
61+
useState<PersonalSkillsMenuState | null>(null);
62+
const isAnchoredOpen = open && anchorRect !== null;
63+
const activeMenuState: PersonalSkillsMenuState | null = isAnchoredOpen
64+
? {
65+
anchorRect,
66+
query,
67+
skills,
68+
isLoading,
69+
isError,
70+
selectedIndex,
71+
}
72+
: null;
73+
const menuState = activeMenuState ?? lastOpenMenuState;
74+
const menuAnchorRect = menuState?.anchorRect ?? null;
75+
const menuSkills = menuState?.skills ?? [];
76+
const menuSelectedIndex = menuState?.selectedIndex ?? -1;
77+
78+
useLayoutEffect(() => {
79+
if (!isAnchoredOpen) {
80+
return;
81+
}
82+
setLastOpenMenuState({
83+
anchorRect,
84+
query,
85+
skills,
86+
isLoading,
87+
isError,
88+
selectedIndex,
89+
});
90+
}, [
91+
anchorRect,
92+
isAnchoredOpen,
93+
isError,
94+
isLoading,
95+
query,
96+
selectedIndex,
97+
skills,
98+
]);
99+
50100
const handleHighlightedValueChange = (value: string) => {
51-
const nextIndex = skills.findIndex((skill) => skill.name === value);
101+
const nextIndex = menuSkills.findIndex((skill) => skill.name === value);
52102
if (nextIndex >= 0) {
53103
onSelectedIndexChange(nextIndex);
54104
}
55105
};
56106

57-
const shouldRender = open && anchorRect;
58-
59107
return (
60108
<Popover
61-
open={Boolean(shouldRender)}
109+
open={isAnchoredOpen}
62110
onOpenChange={(nextOpen) => {
63111
if (!nextOpen) {
64112
onClose();
65113
}
66114
}}
67115
>
68-
{shouldRender && (
116+
{menuAnchorRect && (
69117
<PopoverAnchor asChild>
70118
<span
71119
aria-hidden="true"
72120
style={{
73121
position: "fixed",
74-
top: anchorRect.top,
75-
left: anchorRect.left,
122+
top: menuAnchorRect.top,
123+
left: menuAnchorRect.left,
76124
width: 1,
77-
height: Math.max(anchorRect.height, MIN_ANCHOR_HEIGHT_PX),
125+
height: Math.max(menuAnchorRect.height, MIN_ANCHOR_HEIGHT_PX),
78126
pointerEvents: "none",
79127
}}
80128
/>
@@ -92,26 +140,26 @@ export const PersonalSkillsTriggerMenu = ({
92140
shouldFilter={false}
93141
loop={false}
94142
onValueChange={handleHighlightedValueChange}
95-
value={skills[selectedIndex]?.name ?? ""}
143+
value={menuSkills[menuSelectedIndex]?.name ?? ""}
96144
>
97145
<CommandList className="max-h-72 border-t-0 mobile-full-width-dropdown-scroll-area">
98-
{isLoading ? (
146+
{menuState?.isLoading ? (
99147
<CommandItem value="loading" disabled>
100148
Loading personal skills...
101149
</CommandItem>
102-
) : isError ? (
150+
) : menuState?.isError ? (
103151
<CommandItem value="error" disabled>
104152
Could not load personal skills. Close and type / again to retry.
105153
</CommandItem>
106-
) : skills.length === 0 ? (
154+
) : menuSkills.length === 0 ? (
107155
<CommandEmpty>
108-
{query
156+
{menuState?.query
109157
? "No personal skills match that query."
110158
: "No personal skills found."}
111159
</CommandEmpty>
112160
) : (
113161
<CommandGroup heading="Personal skills">
114-
{skills.map((skill) => (
162+
{menuSkills.map((skill) => (
115163
<CommandItem
116164
key={skill.id}
117165
value={skill.name}

0 commit comments

Comments
 (0)