mirror of
https://github.com/supabase/supabase.git
synced 2026-10-10 20:05:06 +03:00
## I have read the [CONTRIBUTING.md](https://github.com/supabase/supabase/blob/master/CONTRIBUTING.md) file. YES ## What kind of change does this PR introduce? Bug fix. Two `DropdownMenuTrigger`s wrap a `Button` without `asChild`, so each renders a `<button>` inside a `<button>`. One of them also loses its `aria-label`, leaving an icon-only menu trigger with no accessible name. ## What is the current behavior? `DropdownMenuTrigger` forwards to `DropdownMenuPrimitive.Trigger`, which renders its own `<button>` unless `asChild` is set. So this: ```tsx <DropdownMenuTrigger> <Button variant="default" className="px-1" icon={<MoreVertical />} aria-label={`Open actions for ${hook.title}`} /> </DropdownMenuTrigger> ``` produces `<button><button/></button>`, which is invalid HTML, and puts the props on the inner element rather than on the thing that actually opens the menu. Measured by rendering `HookCard` before and after, rather than reasoning about it: | | before | after | | --- | --- | --- | | `container.querySelectorAll('button button').length` | 1 | 0 | | `aria-label` on `[aria-haspopup="menu"]` | `null` | `Open actions for Send Email` | That second row is the part worth caring about. The `aria-label` was written deliberately for a button whose only content is a `MoreVertical` icon, and it lands on the nested inner button instead of the trigger, so a screen reader gets no name for the control it actually operates. Two sites: - `components/interfaces/Auth/Hooks/HookCard.tsx`, the per-hook actions menu. This is the one with the orphaned `aria-label`. - `components/layouts/ProjectLayout/PauseFailedState.tsx`, the overflow menu next to "Download backup". ## What is the new behavior? Both get `asChild`, so the `Button` becomes the trigger. No nesting, and the props land where they were meant to. ## Additional context #48948 fixed exactly this in `RestoreFailedState.tsx`, which sits in the same directory as `PauseFailedState.tsx` and has the same overflow-menu shape. This is that fix applied to the two places it was not. I swept all 4398 `.tsx` files across studio, www, docs, design-system, ui-library, `packages/ui` and `packages/ui-patterns` for any Radix-style trigger (`DropdownMenu`, `Tooltip`, `Popover`, `Dialog`, `Sheet`, `AlertDialog`, `HoverCard`, `Collapsible`, `ContextMenu`, `Menubar`, `Select`, `Tabs`, `Accordion`) that wraps a button-like element without `asChild`. After discarding one false positive in `EdgeFunctionDetails.tsx`, where the `Button` is a sibling of `TabsTrigger` inside `TabsList` rather than its child, these two are the only ones left. So this should be the end of the pattern rather than the start of a series. No test added, matching what #48948 did for the same change. The `asChild` behaviour belongs to Radix, and a test asserting DOM nesting around two JSX attributes would be testing the library. I did verify it the other way round while developing: a throwaway render assertion failed on unmodified master with a nested-button count of 1 and a null trigger `aria-label`, and passed after the change. Happy to commit that assertion if you would rather have it in the suite. Gates: `test:prettier` passes repo wide, `typecheck --filter=studio --force` passes 9/9, `--filter studio run lint:ratchet` reports rules improved, and the tests covering both touched directories pass (18 files, 143 tests, including the `RestoringState` suite that came in with #48948). Freshman contributor. Found this with Claude Code's help by checking whether the `asChild` fix in #48948 had siblings, and I confirmed the nesting and the missing accessible name myself before touching anything. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved dropdown menu trigger behavior in the authentication hooks and project layout interfaces. * Existing buttons now correctly serve as menu triggers without changing available actions or menu behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
190 lines
7.0 KiB
TypeScript
190 lines
7.0 KiB
TypeScript
import { PermissionAction } from '@supabase/shared-types/out/constants'
|
|
import { BookOpen, Check, Edit, MoreVertical, Trash, Webhook } from 'lucide-react'
|
|
import {
|
|
Badge,
|
|
Button,
|
|
DropdownMenu,
|
|
DropdownMenuContent,
|
|
DropdownMenuItem,
|
|
DropdownMenuSeparator,
|
|
DropdownMenuTrigger,
|
|
} from 'ui'
|
|
import { Input } from 'ui-patterns/DataInputs/Input'
|
|
|
|
import { Hook } from './hooks.constants'
|
|
import { DropdownMenuItemTooltip } from '@/components/ui/DropdownMenuItemTooltip'
|
|
import { useAsyncCheckPermissions } from '@/hooks/misc/useCheckPermissions'
|
|
import { DOCS_URL } from '@/lib/constants'
|
|
|
|
interface HookCardProps {
|
|
hook: Hook
|
|
onSelectEdit: () => void
|
|
onSelectDelete: () => void
|
|
}
|
|
|
|
export const HookCard = ({ hook, onSelectEdit, onSelectDelete }: HookCardProps) => {
|
|
const { can: canUpdateAuthHook } = useAsyncCheckPermissions(PermissionAction.AUTH_EXECUTE, '*')
|
|
|
|
return (
|
|
<div className="bg-surface-100 border-default overflow-hidden border shadow-sm px-5 py-4 flex flex-row first:rounded-t-md last:rounded-b-md space-x-4">
|
|
<div>
|
|
<Webhook size={21} strokeWidth="1" />
|
|
</div>
|
|
<div className="flex flex-col grow overflow-y-auto w-full">
|
|
<span className="text-sm text-foreground">{hook.title}</span>
|
|
<span className="text-sm text-foreground-lighter">{hook.subtitle}</span>
|
|
<div className="text-sm flex flex-row space-x-5 py-4">
|
|
{hook.method.type === 'postgres' ? (
|
|
<div className="flex flex-col w-full space-y-2 max-w-xl">
|
|
<div className="flex flex-row items-center">
|
|
<span className="text-foreground-light w-20">Type</span>
|
|
<span className="text-foreground">Postgres function</span>
|
|
</div>
|
|
<div className="flex flex-row items-center">
|
|
<label htmlFor="schema" className="text-foreground-light w-20">
|
|
Schema
|
|
</label>
|
|
<Input
|
|
id="schema"
|
|
title={hook.method.schema}
|
|
copy
|
|
readOnly
|
|
disabled
|
|
containerClassName="flex-1"
|
|
className="font-mono text-xs md:text-xs disabled:text-foreground-light opacity-100"
|
|
value={hook.method.schema}
|
|
/>
|
|
</div>
|
|
<div className="flex flex-row items-center">
|
|
<label htmlFor="functionName" className="text-foreground-light w-20">
|
|
Function
|
|
</label>
|
|
<Input
|
|
id="functionName"
|
|
title={hook.method.functionName}
|
|
copy
|
|
readOnly
|
|
disabled
|
|
containerClassName="flex-1"
|
|
className="font-mono text-xs md:text-xs disabled:text-foreground-light opacity-100"
|
|
value={hook.method.functionName}
|
|
/>
|
|
</div>
|
|
</div>
|
|
) : (
|
|
<div className="flex flex-col w-full space-y-2 max-w-xl">
|
|
<div className="flex flex-row items-center">
|
|
<span className="text-foreground-light w-20">Type</span>
|
|
<span className="text-foreground">HTTPS endpoint</span>
|
|
</div>
|
|
<div className="flex flex-row items-center">
|
|
<label htmlFor="url" className="text-foreground-light w-20">
|
|
Endpoint
|
|
</label>
|
|
<Input
|
|
id="url"
|
|
title={hook.method.url}
|
|
copy
|
|
readOnly
|
|
disabled
|
|
containerClassName="flex-1"
|
|
className="font-mono text-xs md:text-xs disabled:text-foreground-light opacity-100"
|
|
value={hook.method.url}
|
|
/>
|
|
</div>
|
|
<div className="flex flex-row items-center">
|
|
<label htmlFor="secret" className="text-foreground-light w-20">
|
|
Secret
|
|
</label>
|
|
<Input
|
|
id="secret"
|
|
copy
|
|
title={hook.method.secret}
|
|
reveal={true}
|
|
readOnly
|
|
disabled
|
|
containerClassName="flex-1"
|
|
className="font-mono text-xs md:text-xs disabled:text-foreground-light opacity-100"
|
|
value={hook.method.secret}
|
|
/>
|
|
</div>
|
|
</div>
|
|
)}
|
|
</div>
|
|
</div>
|
|
|
|
<div>
|
|
<div className="flex items-center gap-x-2">
|
|
{hook.enabled ? (
|
|
<Badge className="space-x-1" variant="success">
|
|
<div className="h-3.5 w-3.5 bg-brand rounded-full flex justify-center items-center">
|
|
<Check className="h-2 w-2 text-background-overlay " strokeWidth={6} />
|
|
</div>
|
|
<span>Enabled</span>
|
|
</Badge>
|
|
) : (
|
|
<Badge variant="warning">
|
|
<span>Disabled</span>
|
|
</Badge>
|
|
)}
|
|
<DropdownMenu>
|
|
<DropdownMenuTrigger asChild>
|
|
<Button
|
|
variant="default"
|
|
className="px-1"
|
|
icon={<MoreVertical />}
|
|
aria-label={`Open actions for ${hook.title}`}
|
|
/>
|
|
</DropdownMenuTrigger>
|
|
<DropdownMenuContent align="end" className="w-40">
|
|
<DropdownMenuItemTooltip
|
|
className="gap-x-2"
|
|
disabled={!canUpdateAuthHook}
|
|
onClick={onSelectEdit}
|
|
tooltip={{
|
|
content: {
|
|
text: !canUpdateAuthHook
|
|
? 'You need additional permissions to configure auth hooks'
|
|
: undefined,
|
|
side: 'left',
|
|
},
|
|
}}
|
|
>
|
|
<Edit size={12} />
|
|
<span>Edit hook</span>
|
|
</DropdownMenuItemTooltip>
|
|
<DropdownMenuItem asChild className="gap-x-2">
|
|
<a
|
|
target="_blank"
|
|
rel="noreferrer noopener"
|
|
href={`${DOCS_URL}/guides/auth/auth-hooks/${hook.docSlug}`}
|
|
>
|
|
<BookOpen size={12} />
|
|
<span>Documentation</span>
|
|
</a>
|
|
</DropdownMenuItem>
|
|
<DropdownMenuSeparator />
|
|
<DropdownMenuItemTooltip
|
|
className="gap-x-2"
|
|
disabled={!canUpdateAuthHook}
|
|
onClick={onSelectDelete}
|
|
tooltip={{
|
|
content: {
|
|
text: !canUpdateAuthHook
|
|
? 'You need additional permissions to delete auth hooks'
|
|
: undefined,
|
|
side: 'left',
|
|
},
|
|
}}
|
|
>
|
|
<Trash size={12} />
|
|
<span>Delete hook</span>
|
|
</DropdownMenuItemTooltip>
|
|
</DropdownMenuContent>
|
|
</DropdownMenu>
|
|
</div>
|
|
</div>
|
|
</div>
|
|
)
|
|
}
|