mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-24 16:03:02 +00:00
fix(assistant): the window's menu and trip list keep the keyboard in reach
The home chat's trip list opened before its trips had loaded, so the focus stayed on its button and no trip could be reached by keyboard; the first trip now takes the focus once the list is there. The header's menu says whether it is open, and closed by Escape or a pick it hands the focus back to its button, so the next Escape closes the window again. The open chat in the history list is announced as the current one.
This commit is contained in:
@@ -11,7 +11,7 @@ import {
|
||||
Trash2,
|
||||
X,
|
||||
} from 'lucide-react';
|
||||
import { type HTMLAttributes, type MouseEvent, useState } from 'react';
|
||||
import { type HTMLAttributes, type MouseEvent, useRef, useState } from 'react';
|
||||
import { useNavigate } from 'react-router';
|
||||
|
||||
import { useTranslation } from '../../i18n';
|
||||
@@ -79,6 +79,7 @@ export default function AssistantHeader({
|
||||
const [memoryOpen, setMemoryOpen] = useState(false);
|
||||
const navigate = useNavigate();
|
||||
const menu = useContextMenu();
|
||||
const moreRef = useRef<HTMLButtonElement>(null);
|
||||
const [confirming, setConfirming] = useState(false);
|
||||
const threads = thread.threads ?? [];
|
||||
const history = usePopoverList(threads.findIndex((th) => th.id === thread.threadId) + 1);
|
||||
@@ -95,6 +96,14 @@ export default function AssistantHeader({
|
||||
run();
|
||||
};
|
||||
|
||||
// Closed by Escape or a pick while it holds the focus, the menu hands it back to its button, so the keys
|
||||
// go on reaching the window; a click or a scroll elsewhere leaves the focus where it went.
|
||||
const closeMenu = () => {
|
||||
const fromMenu = document.activeElement?.closest('[role="menu"]') != null;
|
||||
menu.close();
|
||||
if (fromMenu) moreRef.current?.focus();
|
||||
};
|
||||
|
||||
const openMenu = (e: MouseEvent) =>
|
||||
menu.open(
|
||||
e,
|
||||
@@ -187,10 +196,12 @@ export default function AssistantHeader({
|
||||
</Tooltip>
|
||||
<Tooltip label={t('assistant.window.menu')}>
|
||||
<button
|
||||
ref={moreRef}
|
||||
type="button"
|
||||
onClick={openMenu}
|
||||
aria-label={t('assistant.window.menu')}
|
||||
aria-haspopup="menu"
|
||||
aria-expanded={menu.menu !== null}
|
||||
className={ICON_BUTTON}
|
||||
>
|
||||
<Ellipsis size={14} aria-hidden="true" />
|
||||
@@ -218,6 +229,7 @@ export default function AssistantHeader({
|
||||
icon={null}
|
||||
label={th.title}
|
||||
active={th.id === thread.threadId}
|
||||
current={th.id === thread.threadId}
|
||||
onClick={() => pick(() => thread.openThread(th.id))}
|
||||
trailing={
|
||||
<span className="flex-shrink-0 text-caption text-content-faint">
|
||||
@@ -234,7 +246,7 @@ export default function AssistantHeader({
|
||||
)}
|
||||
</PopoverListPanel>
|
||||
|
||||
<ContextMenu menu={menu.menu} onClose={menu.close} />
|
||||
<ContextMenu menu={menu.menu} onClose={closeMenu} />
|
||||
{memoryOpen && <AssistantMemoryDialog tripId={tripId} onClose={() => setMemoryOpen(false)} />}
|
||||
<ConfirmDialog
|
||||
isOpen={confirming}
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { ChevronRight, Luggage } from 'lucide-react';
|
||||
import { useEffect, useRef } from 'react';
|
||||
|
||||
import { useTranslation } from '../../i18n';
|
||||
import EmptyState from '../shared/EmptyState';
|
||||
@@ -8,11 +9,17 @@ import { type PickerTrip, useAssistantTripPicker } from './useAssistantTripPicke
|
||||
/**
|
||||
* The user's trips to go on with in one trip's own chat, from the home chat's
|
||||
* head: that chat, its history and its changes belong to the trip. The
|
||||
* running and the next trip lead.
|
||||
* running and the next trip lead. The list opens before the trips are there,
|
||||
* so the focus goes onto the first one once they come.
|
||||
*/
|
||||
export default function AssistantTripPicker({ onPick }: { onPick: (trip: PickerTrip) => void }) {
|
||||
const { t } = useTranslation();
|
||||
const trips = useAssistantTripPicker();
|
||||
const first = useRef<HTMLButtonElement>(null);
|
||||
|
||||
useEffect(() => {
|
||||
first.current?.focus({ preventScroll: true });
|
||||
}, [trips]);
|
||||
|
||||
if (trips === null) {
|
||||
return (
|
||||
@@ -32,9 +39,10 @@ export default function AssistantTripPicker({ onPick }: { onPick: (trip: PickerT
|
||||
return (
|
||||
<div className="flex min-h-0 flex-1 flex-col overflow-y-auto p-2">
|
||||
<ul className="trek-stagger m-0 flex list-none flex-col gap-1 p-0">
|
||||
{trips.map((trip) => (
|
||||
{trips.map((trip, i) => (
|
||||
<li key={trip.id}>
|
||||
<button
|
||||
ref={i === 0 ? first : undefined}
|
||||
type="button"
|
||||
onClick={() => onPick(trip)}
|
||||
className="flex w-full items-center gap-3 rounded-lg px-2.5 py-2 text-start transition-colors hover:bg-surface-hover"
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
/**
|
||||
* FE-AI-WIN-001 to FE-AI-WIN-014
|
||||
* FE-AI-WIN-001 to FE-AI-WIN-016
|
||||
*
|
||||
* The desktop window around the chat: when it shows at all, the button it
|
||||
* opens from, the home chat outside a trip with its way into one trip's chat
|
||||
@@ -238,6 +238,34 @@ describe('AssistantWindow', () => {
|
||||
expect(useAssistantStore.getState().window.open).toBe(true);
|
||||
});
|
||||
|
||||
it("FE-AI-WIN-015: the header's menu says whether it is open, and Escape hands the focus back to its button", async () => {
|
||||
seed({ open: true, minimized: false });
|
||||
render(<AssistantWindow trip={TRIP} />);
|
||||
const more = screen.getByRole('button', { name: 'More options' });
|
||||
expect(more).toHaveAttribute('aria-expanded', 'false');
|
||||
fireEvent.click(more);
|
||||
expect(more).toHaveAttribute('aria-expanded', 'true');
|
||||
fireEvent.keyDown(await screen.findByRole('menu'), { key: 'Escape' });
|
||||
await waitFor(() => expect(screen.queryByRole('menu')).not.toBeInTheDocument());
|
||||
expect(more).toHaveFocus();
|
||||
expect(more).toHaveAttribute('aria-expanded', 'false');
|
||||
// From the button the next Escape is the window's again.
|
||||
fireEvent.keyDown(more, { key: 'Escape' });
|
||||
expect(useAssistantStore.getState().window.open).toBe(false);
|
||||
});
|
||||
|
||||
it("FE-AI-WIN-016: the home chat's trip list takes the focus onto the first trip once the trips are there", async () => {
|
||||
seed({ open: true, minimized: false });
|
||||
render(<AssistantWindow trip={null} />);
|
||||
const button = screen.getByRole('button', { name: 'Chat about one trip' });
|
||||
fireEvent.click(button);
|
||||
// The list's own row, not the chat's "go on in Lisbon" beside it.
|
||||
const lisbon = await screen.findByRole('button', { name: /^Lisbon/ });
|
||||
await waitFor(() => expect(lisbon).toHaveFocus());
|
||||
fireEvent.keyDown(lisbon, { key: 'Escape' });
|
||||
expect(button).toHaveFocus();
|
||||
});
|
||||
|
||||
it('FE-AI-WIN-010: tells the map where it sits, is compact at 400 px wide, and picks days on screen', () => {
|
||||
seed({ open: true, minimized: false });
|
||||
const onFocusDay = vi.fn();
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
// FE-PACKPOP-001 to FE-PACKPOP-008
|
||||
// FE-PACKPOP-001 to FE-PACKPOP-009
|
||||
import { describe, it, expect, vi } from 'vitest'
|
||||
import { useRef } from 'react'
|
||||
import { render, screen, fireEvent } from '../../../tests/helpers/render'
|
||||
@@ -56,6 +56,15 @@ describe('PopoverItem', () => {
|
||||
expect(danger.style.background).toBe('none')
|
||||
})
|
||||
|
||||
it('FE-PACKPOP-009: the entry open now is said to be the current one, not only tinted', () => {
|
||||
render(<>
|
||||
<PopoverItem icon={null} label="Temples" active current onClick={() => {}} />
|
||||
<PopoverItem icon={null} label="Food" onClick={() => {}} />
|
||||
</>)
|
||||
expect(screen.getByRole('button', { name: 'Temples' })).toHaveAttribute('aria-current', 'true')
|
||||
expect(screen.getByRole('button', { name: 'Food' })).not.toHaveAttribute('aria-current')
|
||||
})
|
||||
|
||||
it('FE-PACKPOP-004: the composer button fades while there is nothing to add', () => {
|
||||
expect(composerConfirm(false)).toMatchObject({ opacity: 0.35, cursor: 'default' })
|
||||
expect(composerConfirm(true)).toMatchObject({ opacity: 1, cursor: 'pointer' })
|
||||
|
||||
@@ -7,6 +7,8 @@ interface PopoverItemProps {
|
||||
label: string
|
||||
onClick: () => void
|
||||
active?: boolean
|
||||
/** The entry is the one open now (a chat in a history list), said so to a screen reader too. */
|
||||
current?: boolean
|
||||
danger?: boolean
|
||||
/** A secondary line, such as Add bag, in the faint text colour. */
|
||||
muted?: boolean
|
||||
@@ -14,11 +16,12 @@ interface PopoverItemProps {
|
||||
trailing?: ReactNode
|
||||
}
|
||||
|
||||
export function PopoverItem({ icon, label, onClick, active = false, danger = false, muted = false, trailing }: PopoverItemProps) {
|
||||
export function PopoverItem({ icon, label, onClick, active = false, current = false, danger = false, muted = false, trailing }: PopoverItemProps) {
|
||||
const tone = danger ? '#ef4444' : muted ? 'var(--text-faint)' : 'var(--text-primary)'
|
||||
return (
|
||||
<button type="button"
|
||||
onClick={onClick}
|
||||
aria-current={current || undefined}
|
||||
style={{
|
||||
display: 'flex', alignItems: 'center', gap: 9, width: '100%',
|
||||
padding: '8px 10px', borderRadius: 9, border: 'none', cursor: 'pointer',
|
||||
|
||||
Reference in New Issue
Block a user