mirror of
https://github.com/getpaseo/paseo.git
synced 2026-11-02 22:52:58 +00:00
Keep model picker rows free of nested buttons on web (#5404)
* Keep the model row's profile action out of the row button on web On web the model row renders as a <button>, and the create/edit profile action inside it rendered a second <button>, so React logged "<button> cannot be a descendant of <button>" for every row. The action now sits beside the row button inside the same hover boundary, positioned over a slot the row reserves, so layout and hover reveal are unchanged. Closes #5385 * Check the create-profile action leaves the composer's model unchanged --------- Co-authored-by: paseo-bot[bot] <266920839+paseo-bot[bot]@users.noreply.github.com>
This commit is contained in:
@@ -6,13 +6,16 @@ import {
|
||||
expectAgentProfilesEmptyPrompt,
|
||||
expectProfileEditTooltip,
|
||||
expectComposerMode,
|
||||
expectCreateProfileFromModelRow,
|
||||
expectComposerModel,
|
||||
expectModelRowProfileActionBesideRow,
|
||||
expectModelRowSelected,
|
||||
expectProfileEditIsPencilOnly,
|
||||
expectProfileVisibleForProvider,
|
||||
openModelPicker,
|
||||
openAgentProfilesFromEmptyPrompt,
|
||||
seedAgentProfiles,
|
||||
selectModelRow,
|
||||
} from "../support/helpers/agent-profiles";
|
||||
import { expectWorkspaceAgentConfiguration } from "../support/helpers/command-center-agent-controls";
|
||||
import { expectComposerVisible } from "../support/helpers/composer";
|
||||
@@ -115,4 +118,34 @@ test.describe("Agent profiles in the model picker", () => {
|
||||
await seed.restore();
|
||||
}
|
||||
});
|
||||
|
||||
test("a model row and its create-profile action are separate buttons", async ({ page }) => {
|
||||
const seed = await seedAgentProfiles([]);
|
||||
const workspace = await seedMockAgentWorkspace({
|
||||
repoPrefix: "agent-profiles-row-action-",
|
||||
title: "Agent profiles row action",
|
||||
model: "ten-second-stream",
|
||||
modeId: "load-test",
|
||||
});
|
||||
|
||||
try {
|
||||
const oneMinute = {
|
||||
provider: "mock",
|
||||
modelId: "one-minute-stream",
|
||||
modelLabel: "One minute stream",
|
||||
};
|
||||
await openAgentRoute(page, workspace);
|
||||
await expectComposerVisible(page);
|
||||
await openModelPicker(page);
|
||||
await expectModelRowProfileActionBesideRow(page, oneMinute);
|
||||
await expectCreateProfileFromModelRow(page, oneMinute);
|
||||
await expectComposerModel(page, "Ten second stream");
|
||||
await openModelPicker(page);
|
||||
await selectModelRow(page, oneMinute);
|
||||
await expectComposerModel(page, oneMinute.modelLabel);
|
||||
} finally {
|
||||
await workspace.cleanup();
|
||||
await seed.restore();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -482,6 +482,50 @@ export async function expectModelRowSelected(
|
||||
).toHaveAttribute("aria-selected", "true", { timeout: 30_000 });
|
||||
}
|
||||
|
||||
function modelRow(page: Page, input: { provider: string; modelId: string }): Locator {
|
||||
return pickerViewport(page).getByTestId(`model-row-${input.provider}-${input.modelId}`);
|
||||
}
|
||||
|
||||
/**
|
||||
* A model row's profile action is its own button. On web, a button inside the
|
||||
* row's button is invalid HTML that React reports on every render.
|
||||
*/
|
||||
export async function expectModelRowProfileActionBesideRow(
|
||||
page: Page,
|
||||
input: { provider: string; modelId: string },
|
||||
): Promise<void> {
|
||||
const row = modelRow(page, input);
|
||||
await expect(row).toBeVisible({ timeout: 30_000 });
|
||||
await expect(pickerViewport(page).locator("button button")).toHaveCount(0);
|
||||
await expect(row.getByRole("button")).toHaveCount(0);
|
||||
}
|
||||
|
||||
/** The hover-revealed plus on an unprofiled model row opens a seeded profile form. */
|
||||
export async function expectCreateProfileFromModelRow(
|
||||
page: Page,
|
||||
input: { provider: string; modelId: string; modelLabel: string },
|
||||
): Promise<void> {
|
||||
await modelRow(page, input).hover();
|
||||
await pickerViewport(page)
|
||||
.getByTestId(`model-create-profile-${input.provider}-${input.modelId}`)
|
||||
.click();
|
||||
const modal = editModal(page);
|
||||
await expect(modal).toBeVisible({ timeout: 30_000 });
|
||||
await expect(modal.getByRole("textbox", { name: "Name", exact: true })).toHaveValue(
|
||||
input.modelLabel,
|
||||
);
|
||||
await expect(modal.getByRole("button", { name: `Model (${input.modelLabel})` })).toBeVisible();
|
||||
await modal.getByRole("button", { name: "Cancel", exact: true }).click();
|
||||
await expect(modal).toHaveCount(0, { timeout: 30_000 });
|
||||
}
|
||||
|
||||
export async function selectModelRow(
|
||||
page: Page,
|
||||
input: { provider: string; modelId: string },
|
||||
): Promise<void> {
|
||||
await modelRow(page, input).click();
|
||||
}
|
||||
|
||||
// ─── Cross-provider model search ───────────────────────────────────────────
|
||||
|
||||
export async function searchAllModels(page: Page, query: string): Promise<void> {
|
||||
|
||||
@@ -625,13 +625,6 @@ function ModelRowProfileAction({
|
||||
}) {
|
||||
const isCompact = useIsCompactFormFactor();
|
||||
const visible = hovered || isNative || isCompact;
|
||||
const handlePress = useCallback(
|
||||
(event: GestureResponderEvent) => {
|
||||
event.stopPropagation();
|
||||
onPress();
|
||||
},
|
||||
[onPress],
|
||||
);
|
||||
const pressableStyle = useCallback(
|
||||
({ hovered: buttonHovered, pressed }: PressableStateCallbackType & { hovered?: boolean }) => [
|
||||
styles.rowIconButton,
|
||||
@@ -645,7 +638,7 @@ function ModelRowProfileAction({
|
||||
<Tooltip delayDuration={250} enabledOnDesktop enabledOnMobile={false}>
|
||||
<TooltipTrigger asChild>
|
||||
<Pressable
|
||||
onPress={handlePress}
|
||||
onPress={onPress}
|
||||
hitSlop={8}
|
||||
style={pressableStyle}
|
||||
pointerEvents={visible ? "auto" : "none"}
|
||||
@@ -778,7 +771,6 @@ function ModelRow({
|
||||
const pressableStyle = useCallback(
|
||||
({ hovered, pressed }: PressableStateCallbackType & { hovered?: boolean }) => [
|
||||
styles.browserRow,
|
||||
styles.browserModelRow,
|
||||
Boolean(hovered) && styles.browserRowHovered,
|
||||
pressed && styles.browserRowPressed,
|
||||
],
|
||||
@@ -787,7 +779,7 @@ function ModelRow({
|
||||
|
||||
return (
|
||||
<View
|
||||
style={styles.modelRowHoverBoundary}
|
||||
style={[styles.modelRowHoverBoundary, styles.browserModelRow]}
|
||||
onPointerEnter={handlePointerEnter}
|
||||
onPointerLeave={handlePointerLeave}
|
||||
>
|
||||
@@ -815,10 +807,17 @@ function ModelRow({
|
||||
<ThemedCheck size={ICON_SIZE.sm} uniProps={foregroundMutedMapping} />
|
||||
) : null}
|
||||
</View>
|
||||
{profileAction}
|
||||
{profileAction ? <View style={styles.rowIconButton} /> : null}
|
||||
</View>
|
||||
</View>
|
||||
</ModelBrowserPressable>
|
||||
{/* The row renders a <button> on web, so its profile action sits beside it,
|
||||
over the slot reserved above, rather than inside it. */}
|
||||
{profileAction ? (
|
||||
<View style={styles.modelRowProfileActionSlot} pointerEvents="box-none">
|
||||
{profileAction}
|
||||
</View>
|
||||
) : null}
|
||||
</View>
|
||||
);
|
||||
}
|
||||
@@ -1590,6 +1589,13 @@ const styles = StyleSheet.create((theme) => ({
|
||||
modelRowHoverBoundary: {
|
||||
position: "relative",
|
||||
},
|
||||
modelRowProfileActionSlot: {
|
||||
position: "absolute",
|
||||
top: 0,
|
||||
bottom: 0,
|
||||
right: isWeb ? theme.spacing[3] : theme.spacing[6],
|
||||
justifyContent: "center",
|
||||
},
|
||||
browserModelRow: isWeb ? {} : { marginBottom: theme.spacing[1] },
|
||||
browserRowHovered: {
|
||||
backgroundColor: theme.colors.surface1,
|
||||
|
||||
Reference in New Issue
Block a user