Skip to content

Commit 1428004

Browse files
vignesh191vignesh191
authored
webui : [ChatFormActionAdd][a11y] fix accessibility issues in add menu trigger and items (ggml-org#22736)
* fix tab order on attach button, and dont focus on disabled mennu item * add a11y tests
1 parent 366c5e2 commit 1428004

3 files changed

Lines changed: 117 additions & 44 deletions

File tree

‎tools/ui/src/lib/components/app/chat/ChatForm/ChatFormActions/ChatFormActionAdd/ChatFormActionAddDropdown.svelte‎

Lines changed: 66 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,14 @@
11
<script lang="ts">
2-
import type { Snippet } from 'svelte';
2+
import { Plus } from '@lucide/svelte';
33
import * as DropdownMenu from '$lib/components/ui/dropdown-menu';
44
import * as Tooltip from '$lib/components/ui/tooltip';
5+
import { buttonVariants } from '$lib/components/ui/button';
6+
import { cn } from '$lib/components/ui/utils';
57
import {
68
ATTACHMENT_FILE_ITEMS,
79
ATTACHMENT_EXTRA_ITEMS,
810
ATTACHMENT_MCP_ITEMS,
11+
ATTACHMENT_TOOLTIP_TEXT,
912
TOOLTIP_DELAY_DURATION
1013
} from '$lib/constants';
1114
import { AttachmentMenuItemId } from '$lib/enums';
@@ -28,7 +31,6 @@
2831
onMcpPromptClick?: () => void;
2932
onMcpSettingsClick?: () => void;
3033
onMcpResourcesClick?: () => void;
31-
trigger: Snippet<[{ disabled: boolean }]>;
3234
}
3335
3436
let {
@@ -42,8 +44,7 @@
4244
onSystemPromptClick,
4345
onMcpPromptClick,
4446
onMcpSettingsClick,
45-
onMcpResourcesClick,
46-
trigger
47+
onMcpResourcesClick
4748
}: Props = $props();
4849
4950
let dropdownOpen = $state(false);
@@ -69,9 +70,28 @@
6970

7071
<div class="flex items-center gap-1 {className}">
7172
<DropdownMenu.Root bind:open={dropdownOpen}>
72-
<DropdownMenu.Trigger name="Attach files" {disabled}>
73-
{@render trigger({ disabled })}
74-
</DropdownMenu.Trigger>
73+
<Tooltip.Root>
74+
<Tooltip.Trigger>
75+
{#snippet child({ props })}
76+
<DropdownMenu.Trigger
77+
{...props}
78+
class={cn(
79+
buttonVariants({ variant: 'secondary' }),
80+
'file-upload-button h-8 w-8 cursor-pointer rounded-full p-0'
81+
)}
82+
{disabled}
83+
>
84+
<span class="sr-only">{ATTACHMENT_TOOLTIP_TEXT}</span>
85+
86+
<Plus class="h-4 w-4" />
87+
</DropdownMenu.Trigger>
88+
{/snippet}
89+
</Tooltip.Trigger>
90+
91+
<Tooltip.Content>
92+
<p>{ATTACHMENT_TOOLTIP_TEXT}</p>
93+
</Tooltip.Content>
94+
</Tooltip.Root>
7595

7696
<DropdownMenu.Content align="start" class="w-48">
7797
{#each ATTACHMENT_FILE_ITEMS as item (item.id)}
@@ -87,15 +107,16 @@
87107
</DropdownMenu.Item>
88108
{:else if item.disabledTooltip}
89109
<Tooltip.Root delayDuration={TOOLTIP_DELAY_DURATION}>
90-
<Tooltip.Trigger class="w-full">
91-
<DropdownMenu.Item
92-
class="{item.class ?? ''} flex cursor-pointer items-center gap-2"
93-
disabled
94-
>
95-
<item.icon class="h-4 w-4" />
96-
97-
<span>{item.label}</span>
98-
</DropdownMenu.Item>
110+
<Tooltip.Trigger tabindex={-1}>
111+
{#snippet child({ props })}
112+
<div {...props} class="cursor-default">
113+
<DropdownMenu.Item class="{item.class ?? ''} flex items-center gap-2" disabled>
114+
<item.icon class="h-4 w-4" />
115+
116+
<span>{item.label}</span>
117+
</DropdownMenu.Item>
118+
</div>
119+
{/snippet}
99120
</Tooltip.Trigger>
100121

101122
<Tooltip.Content side="right">
@@ -107,20 +128,23 @@
107128

108129
{#if !attachmentMenu.isItemEnabled('hasVisionModality')}
109130
<Tooltip.Root delayDuration={TOOLTIP_DELAY_DURATION}>
110-
<Tooltip.Trigger class="w-full">
111-
<DropdownMenu.Item
112-
class="flex cursor-pointer items-center gap-2"
113-
onclick={attachmentMenu.callbacks.onFileUpload}
114-
>
115-
{@const pdfItem = ATTACHMENT_FILE_ITEMS.find(
116-
(i) => i.id === AttachmentMenuItemId.PDF
117-
)}
118-
{#if pdfItem}
119-
<pdfItem.icon class="h-4 w-4" />
120-
121-
<span>{pdfItem.label}</span>
122-
{/if}
123-
</DropdownMenu.Item>
131+
<Tooltip.Trigger>
132+
{#snippet child({ props })}
133+
<DropdownMenu.Item
134+
{...props}
135+
class="flex cursor-pointer items-center gap-2"
136+
onclick={attachmentMenu.callbacks.onFileUpload}
137+
>
138+
{@const pdfItem = ATTACHMENT_FILE_ITEMS.find(
139+
(i) => i.id === AttachmentMenuItemId.PDF
140+
)}
141+
{#if pdfItem}
142+
<pdfItem.icon class="h-4 w-4" />
143+
144+
<span>{pdfItem.label}</span>
145+
{/if}
146+
</DropdownMenu.Item>
147+
{/snippet}
124148
</Tooltip.Trigger>
125149

126150
<Tooltip.Content side="right">
@@ -134,15 +158,18 @@
134158
{#each ATTACHMENT_EXTRA_ITEMS as item (item.id)}
135159
{#if item.id === AttachmentMenuItemId.SYSTEM_MESSAGE}
136160
<Tooltip.Root delayDuration={TOOLTIP_DELAY_DURATION}>
137-
<Tooltip.Trigger class="w-full">
138-
<DropdownMenu.Item
139-
class="flex cursor-pointer items-center gap-2"
140-
onclick={() => attachmentMenu.callbacks[item.action]()}
141-
>
142-
<item.icon class="h-4 w-4" />
143-
144-
<span>{item.label}</span>
145-
</DropdownMenu.Item>
161+
<Tooltip.Trigger>
162+
{#snippet child({ props })}
163+
<DropdownMenu.Item
164+
{...props}
165+
class="flex cursor-pointer items-center gap-2"
166+
onclick={() => attachmentMenu.callbacks[item.action]()}
167+
>
168+
<item.icon class="h-4 w-4" />
169+
170+
<span>{item.label}</span>
171+
</DropdownMenu.Item>
172+
{/snippet}
146173
</Tooltip.Trigger>
147174

148175
<Tooltip.Content side="right">

‎tools/ui/src/lib/components/app/chat/ChatForm/ChatFormActions/ChatFormActionAdd/ChatFormActionsAdd.svelte‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,5 @@
6060
{onMcpResourcesClick}
6161
{onMcpSettingsClick}
6262
{onSystemPromptClick}
63-
>
64-
{#snippet trigger()}
65-
<ChatFormActionAddButton {disabled} />
66-
{/snippet}
67-
</ChatFormActionAddDropdown>
63+
/>
6864
{/if}

‎tools/ui/tests/stories/ChatScreenForm.a11y.stories.svelte‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
<script module lang="ts">
2+
import { defineMeta } from '@storybook/addon-svelte-csf';
3+
import ChatScreenForm from '$lib/components/app/chat/ChatScreen/ChatScreenForm.svelte';
4+
import { expect, screen, waitFor } from 'storybook/test';
5+
import { ATTACHMENT_TOOLTIP_TEXT } from '$lib/constants';
6+
7+
const { Story } = defineMeta({
8+
title: 'Components/ChatScreen/ChatScreenForm/Accessibility',
9+
component: ChatScreenForm,
10+
parameters: {
11+
layout: 'centered'
12+
},
13+
tags: ['!dev']
14+
});
15+
</script>
16+
17+
<Story
18+
name="AddButtonSingleTabStop"
19+
args={{ class: 'max-w-[56rem] w-[calc(100vw-2rem)]' }}
20+
play={async ({ canvas, userEvent }) => {
21+
const textarea = await canvas.findByRole('textbox');
22+
await userEvent.clear(textarea);
23+
await userEvent.type(textarea, 'What is the meaning of life?');
24+
25+
const trigger = await canvas.findByRole('button', { name: ATTACHMENT_TOOLTIP_TEXT });
26+
27+
trigger.focus();
28+
await expect(trigger).toHaveFocus();
29+
30+
await userEvent.tab();
31+
32+
await expect(trigger).not.toHaveFocus();
33+
}}
34+
/>
35+
36+
<Story
37+
name="AddDropdownFocusesFirstEnabled"
38+
args={{ class: 'max-w-[56rem] w-[calc(100vw-2rem)]' }}
39+
play={async ({ canvas, userEvent }) => {
40+
const trigger = await canvas.findByRole('button', { name: ATTACHMENT_TOOLTIP_TEXT });
41+
42+
trigger.focus();
43+
await userEvent.keyboard('{Enter}');
44+
await screen.findByRole('menu');
45+
46+
await waitFor(() => {
47+
expect(document.activeElement).toHaveTextContent('Text Files');
48+
});
49+
}}
50+
/>

0 commit comments

Comments
 (0)