Skip to content

webui : [ChatFormActionAdd][a11y] fix accessibility issues in add menu trigger and items #22736

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
allozaur merged 4 commits into from
May 16, 2026
+117 −44
Merged

webui : [ChatFormActionAdd][a11y] fix accessibility issues in add menu trigger and items #22736

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5716a13
fix tab order on attach button, and dont focus on disabled mennu item
vignesh191 May 6, 2026
618ac9c
merge
vignesh191 May 6, 2026
9e5e7b1
add a11y tests
vignesh191 May 7, 2026
3248818
Merge branch 'master' of github.com:ggml-org/llama.cpp into attachMen…
vignesh191 May 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 66 additions & 39 deletions ...ents/app/chat/ChatForm/ChatFormActions/ChatFormActionAdd/ChatFormActionAddDropdown.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
<script lang="ts">
import type { Snippet } from 'svelte';
import { Plus } from '@lucide/svelte';
import * as DropdownMenu from '$lib/components/ui/dropdown-menu';
import * as Tooltip from '$lib/components/ui/tooltip';
import { buttonVariants } from '$lib/components/ui/button';
import { cn } from '$lib/components/ui/utils';

@allozaur allozaur May 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't need to use cn anymore, as Svelte 5 supports Arrays inside of classes

@vignesh191 vignesh191 May 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did a little digging, and I think we do need it here. Without it, the "+" button renders as a square (rounded-md):

image

buttonVariants looks like it includes the rounded-md. We manually added rounded-full here. Without the cn, both of these classses get added and rounded-md takes styling precedence 🤷 . It seems like cn is doing meaningful work here merging the tw classes

import {
ATTACHMENT_FILE_ITEMS,
ATTACHMENT_EXTRA_ITEMS,
ATTACHMENT_MCP_ITEMS,
ATTACHMENT_TOOLTIP_TEXT,
TOOLTIP_DELAY_DURATION
} from '$lib/constants';
import { AttachmentMenuItemId } from '$lib/enums';
Expand All @@ -28,7 +31,6 @@
onMcpPromptClick?: () => void;
onMcpSettingsClick?: () => void;
onMcpResourcesClick?: () => void;
trigger: Snippet<[{ disabled: boolean }]>;
}

let {
Expand All @@ -42,8 +44,7 @@
onSystemPromptClick,
onMcpPromptClick,
onMcpSettingsClick,
onMcpResourcesClick,
trigger
onMcpResourcesClick
}: Props = $props();

let dropdownOpen = $state(false);
Expand All @@ -69,9 +70,28 @@

<div class="flex items-center gap-1 {className}">
<DropdownMenu.Root bind:open={dropdownOpen}>
<DropdownMenu.Trigger name="Attach files" {disabled}>
{@render trigger({ disabled })}
</DropdownMenu.Trigger>
<Tooltip.Root>
<Tooltip.Trigger>
{#snippet child({ props })}
<DropdownMenu.Trigger
{...props}
class={cn(
buttonVariants({ variant: 'secondary' }),
'file-upload-button h-8 w-8 cursor-pointer rounded-full p-0'
)}
{disabled}
>
<span class="sr-only">{ATTACHMENT_TOOLTIP_TEXT}</span>

<Plus class="h-4 w-4" />
</DropdownMenu.Trigger>
{/snippet}
</Tooltip.Trigger>

<Tooltip.Content>
<p>{ATTACHMENT_TOOLTIP_TEXT}</p>
</Tooltip.Content>
</Tooltip.Root>

<DropdownMenu.Content align="start" class="w-48">
{#each ATTACHMENT_FILE_ITEMS as item (item.id)}
Expand All @@ -87,15 +107,16 @@
</DropdownMenu.Item>
{:else if item.disabledTooltip}
<Tooltip.Root delayDuration={TOOLTIP_DELAY_DURATION}>
<Tooltip.Trigger class="w-full">
<DropdownMenu.Item
class="{item.class ?? ''} flex cursor-pointer items-center gap-2"
disabled
>
<item.icon class="h-4 w-4" />

<span>{item.label}</span>
</DropdownMenu.Item>
<Tooltip.Trigger tabindex={-1}>
{#snippet child({ props })}
<div {...props} class="cursor-default">
Comment thread
allozaur marked this conversation as resolved.
<DropdownMenu.Item class="{item.class ?? ''} flex items-center gap-2" disabled>
<item.icon class="h-4 w-4" />

<span>{item.label}</span>
</DropdownMenu.Item>
</div>
{/snippet}
</Tooltip.Trigger>

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

{#if !attachmentMenu.isItemEnabled('hasVisionModality')}
<Tooltip.Root delayDuration={TOOLTIP_DELAY_DURATION}>
<Tooltip.Trigger class="w-full">
<DropdownMenu.Item
class="flex cursor-pointer items-center gap-2"
onclick={attachmentMenu.callbacks.onFileUpload}
>
{@const pdfItem = ATTACHMENT_FILE_ITEMS.find(
(i) => i.id === AttachmentMenuItemId.PDF
)}
{#if pdfItem}
<pdfItem.icon class="h-4 w-4" />

<span>{pdfItem.label}</span>
{/if}
</DropdownMenu.Item>
<Tooltip.Trigger>
{#snippet child({ props })}
<DropdownMenu.Item
{...props}
class="flex cursor-pointer items-center gap-2"
onclick={attachmentMenu.callbacks.onFileUpload}
>
{@const pdfItem = ATTACHMENT_FILE_ITEMS.find(
(i) => i.id === AttachmentMenuItemId.PDF
)}
{#if pdfItem}
<pdfItem.icon class="h-4 w-4" />

<span>{pdfItem.label}</span>
{/if}
</DropdownMenu.Item>
{/snippet}
</Tooltip.Trigger>

<Tooltip.Content side="right">
Expand All @@ -134,15 +158,18 @@
{#each ATTACHMENT_EXTRA_ITEMS as item (item.id)}
{#if item.id === AttachmentMenuItemId.SYSTEM_MESSAGE}
<Tooltip.Root delayDuration={TOOLTIP_DELAY_DURATION}>
<Tooltip.Trigger class="w-full">
<DropdownMenu.Item
class="flex cursor-pointer items-center gap-2"
onclick={() => attachmentMenu.callbacks[item.action]()}
>
<item.icon class="h-4 w-4" />

<span>{item.label}</span>
</DropdownMenu.Item>
<Tooltip.Trigger>
{#snippet child({ props })}
<DropdownMenu.Item
{...props}
class="flex cursor-pointer items-center gap-2"
onclick={() => attachmentMenu.callbacks[item.action]()}
>
<item.icon class="h-4 w-4" />

<span>{item.label}</span>
</DropdownMenu.Item>
{/snippet}
</Tooltip.Trigger>

<Tooltip.Content side="right">
Expand Down
6 changes: 1 addition & 5 deletions .../components/app/chat/ChatForm/ChatFormActions/ChatFormActionAdd/ChatFormActionsAdd.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,5 @@
{onMcpResourcesClick}
{onMcpSettingsClick}
{onSystemPromptClick}
>
{#snippet trigger()}
<ChatFormActionAddButton {disabled} />
{/snippet}
</ChatFormActionAddDropdown>
/>
{/if}
50 changes: 50 additions & 0 deletions tools/ui/tests/stories/ChatScreenForm.a11y.stories.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
<script module lang="ts">
import { defineMeta } from '@storybook/addon-svelte-csf';
import ChatScreenForm from '$lib/components/app/chat/ChatScreen/ChatScreenForm.svelte';
import { expect, screen, waitFor } from 'storybook/test';
import { ATTACHMENT_TOOLTIP_TEXT } from '$lib/constants';

const { Story } = defineMeta({
title: 'Components/ChatScreen/ChatScreenForm/Accessibility',
component: ChatScreenForm,
parameters: {
layout: 'centered'
},
tags: ['!dev']
Comment thread
allozaur marked this conversation as resolved.
});
</script>

<Story
name="AddButtonSingleTabStop"
args={{ class: 'max-w-[56rem] w-[calc(100vw-2rem)]' }}
play={async ({ canvas, userEvent }) => {
const textarea = await canvas.findByRole('textbox');
await userEvent.clear(textarea);
await userEvent.type(textarea, 'What is the meaning of life?');

const trigger = await canvas.findByRole('button', { name: ATTACHMENT_TOOLTIP_TEXT });

trigger.focus();
await expect(trigger).toHaveFocus();

await userEvent.tab();

await expect(trigger).not.toHaveFocus();
}}
/>

<Story
name="AddDropdownFocusesFirstEnabled"
args={{ class: 'max-w-[56rem] w-[calc(100vw-2rem)]' }}
play={async ({ canvas, userEvent }) => {
const trigger = await canvas.findByRole('button', { name: ATTACHMENT_TOOLTIP_TEXT });

trigger.focus();
await userEvent.keyboard('{Enter}');
await screen.findByRole('menu');

await waitFor(() => {
expect(document.activeElement).toHaveTextContent('Text Files');
});
}}
/>