Minor adjustments

Author: alex-rawlings-yyc

REVIEW.md

@@ -18,6 +18,10 @@ Before reporting any documentation as missing, open the file and confirm the JSD
 
 Keyboard accessibility is planned but not yet implemented. Do not flag missing `tabIndex` attributes, absent `aria-*` roles, or gaps in focus management as issues — these will be addressed in a dedicated pass once the core interaction model is stable.
 
+## Buttons inside the TokenChip label
+
+The `<label>` in [src/components/TokenChip.tsx](src/components/TokenChip.tsx) contains a `<button>` (the morpheme trigger) when morphology is shown, which technically violates the HTML content model for `<label>` (no labelable descendants other than the labeled control). This is **intentional and already handled**: the explicit `htmlFor` binding to the gloss input takes precedence over implicit control resolution in all browsers, and the label's mouse-down handler explicitly routes focus around inputs and buttons. The comment above the label in that file documents the reasoning. Do not flag this as a spec violation or suggest restructuring the markup — moving the morpheme row outside the label would break click-to-focus on the chip body.
+
 ## Mock cleanup in tests
 
 [jest.config.ts](jest.config.ts) sets both `resetMocks: true` and `restoreMocks: true`. This means every `jest.spyOn(...)` is automatically restored to its original implementation after each test — tests do **not** need a manual `mockRestore()` or `jest.restoreAllMocks()` in `afterEach` for spies. Do not flag spies as leaking or suggest adding cleanup for them.

__mocks__/platform-bible-react.tsx

@@ -365,8 +365,11 @@ export function PopoverAnchor({
  * @param props - Component props.
  * @param props.children - Panel content.
  * @param props.className - CSS class names forwarded to the div.
- * @param props.onEscapeKeyDown - Called when Escape is pressed inside the content.
- * @param props.onInteractOutside - Called when the sentinel outside button is clicked.
+ * @param props.onEscapeKeyDown - Called with the native `KeyboardEvent` when Escape is pressed
+ *   inside the content, matching Radix's signature.
+ * @param props.onInteractOutside - Called with a `CustomEvent` carrying the original pointer event
+ *   in `detail.originalEvent` when the sentinel outside button is clicked, matching the shape of
+ *   Radix's `PointerDownOutsideEvent`.
  * @param props.onOpenAutoFocus - Called once on mount with a plain `Event`.
  * @param props.onClick - Click handler forwarded to the div.
  * @param props.onMouseDown - Mouse-down handler forwarded to the div.
@@ -385,8 +388,8 @@ export function PopoverContent({
   className?: string;
   align?: 'start' | 'center' | 'end';
   sideOffset?: number;
-  onEscapeKeyDown?: () => void;
-  onInteractOutside?: () => void;
+  onEscapeKeyDown?: (event: KeyboardEvent) => void;
+  onInteractOutside?: (event: CustomEvent) => void;
   onOpenAutoFocus?: (event: Event) => void;
   onClick?: MouseEventHandler<HTMLDivElement>;
   onMouseDown?: MouseEventHandler<HTMLDivElement>;
@@ -403,7 +406,7 @@ export function PopoverContent({
       data-testid="popover-content"
       onClick={onClick}
       onKeyDown={(e) => {
-        if (e.key === 'Escape') onEscapeKeyDown?.();
+        if (e.key === 'Escape') onEscapeKeyDown?.(e.nativeEvent);
       }}
       onMouseDown={onMouseDown}
     >
@@ -412,7 +415,13 @@ export function PopoverContent({
         <button
           data-testid="popover-outside"
           type="button"
-          onClick={() => onInteractOutside()}
+          onClick={(e) =>
+            onInteractOutside(
+              new CustomEvent('dismissableLayer.pointerDownOutside', {
+                detail: { originalEvent: e.nativeEvent },
+              }),
+            )
+          }
         >
           outside
         </button>

src/__tests__/components/PhraseBox.test.tsx

@@ -336,6 +336,7 @@ describe('PhraseBox', () => {
     if (box instanceof HTMLElement) box.focus();
     await userEvent.keyboard('{Enter}');
 
+    expect(screen.getByRole('textbox', { name: 'Gloss for Hello' })).toHaveFocus();
     expect(screen.getByRole('textbox', { name: 'Gloss for morpheme Hello' })).not.toHaveFocus();
   });
 

src/__tests__/components/TokenChip.test.tsx

@@ -559,6 +559,31 @@ describe('TokenChip', () => {
       expect(focusSpy).not.toHaveBeenCalled();
     });
 
+    it('does not reopen the popover when showMorphology is toggled off and back on', async () => {
+      const { rerender } = render(
+        <AnalysisStoreProvider analysisLanguage="und">
+          <TokenChip {...requiredProps()} showMorphology />
+        </AnalysisStoreProvider>,
+      );
+      await userEvent.click(
+        screen.getByRole('button', { name: 'Define morpheme breakdown for hello' }),
+      );
+      expect(screen.getByTestId('morpheme-popover')).toBeInTheDocument();
+      // Toggling morphology off unmounts the popover tree; the open state must not survive and
+      // resurrect the popover when morphology comes back.
+      rerender(
+        <AnalysisStoreProvider analysisLanguage="und">
+          <TokenChip {...requiredProps()} showMorphology={false} />
+        </AnalysisStoreProvider>,
+      );
+      rerender(
+        <AnalysisStoreProvider analysisLanguage="und">
+          <TokenChip {...requiredProps()} showMorphology />
+        </AnalysisStoreProvider>,
+      );
+      expect(screen.queryByTestId('morpheme-popover')).not.toBeInTheDocument();
+    });
+
     it('closes the popover via onClose', async () => {
       render(
         <AnalysisStoreProvider analysisLanguage="und">

src/components/TokenChip.tsx

@@ -76,6 +76,13 @@ export function TokenChip({
     setDraft(committedGloss);
   }, [committedGloss]);
 
+  // The popover tree unmounts with the morpheme row when showMorphology turns off, but this state
+  // lives on the chip and would survive — silently reopening the popover when morphology is shown
+  // again. Clear it so hiding morphology also closes the popover.
+  useEffect(() => {
+    if (!showMorphology) setPopoverOpen(false);
+  }, [showMorphology]);
+
   const handleMouseDown: MouseEventHandler<HTMLInputElement> = (e) => {
     // Prevent the browser's built-in focus-and-scroll so only the React-controlled
     // smooth scrollIntoView fires. We re-focus manually with preventScroll instead.