fix: expose first suggestion as active descendant - #2965
Conversation
|
@pupuking723 is attempting to deploy a commit to the TypeCell Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe change adds a shared helper for suggestion-menu item IDs. Both suggestion-menu wrappers use it for ChangesSuggestion menu accessibility
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/react/src/components/SuggestionMenu/SuggestionMenu.test.tsx`:
- Around line 44-49: Extend the SuggestionMenu tests around
getSuggestionMenuItemId to cover wrapper-level aria-activedescendant behavior
for both wrapper components. Verify index 0, a later index, and undefined, and
confirm the attribute is removed during cleanup through each wrapper.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: fbc24cb8-93bb-44e6-82d6-4b56e9f744a8
📒 Files selected for processing (4)
packages/react/src/components/SuggestionMenu/GridSuggestionMenu/GridSuggestionMenuWrapper.tsxpackages/react/src/components/SuggestionMenu/SuggestionMenu.test.tsxpackages/react/src/components/SuggestionMenu/SuggestionMenuWrapper.tsxpackages/react/src/components/SuggestionMenu/getSuggestionMenuItemId.ts
|
|
||
| it("returns an active descendant id for the first suggestion", () => { | ||
| expect(getSuggestionMenuItemId(0)).toBe("bn-suggestion-menu-item-0"); | ||
| expect(getSuggestionMenuItemId(2)).toBe("bn-suggestion-menu-item-2"); | ||
| expect(getSuggestionMenuItemId(undefined)).toBeUndefined(); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
target="packages/react/src/components/SuggestionMenu/SuggestionMenu.test.tsx"
printf '%s\n' '--- target file map ---'
ast-grep outline "$target" --view expanded
printf '%s\n' '--- target test file ---'
cat -n "$target"
printf '%s\n' '--- related SuggestionMenu files ---'
fd -i 'SuggestionMenu' packages/react/src/components | sort
printf '%s\n' '--- wrapper definitions and usages ---'
rg -n -C 4 'SuggestionMenuWrapper|GridSuggestionMenuWrapper|aria-activedescendant|getSuggestionMenuItemId' packages/react/srcRepository: TypeCellOS/BlockNote
Length of output: 20042
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- all active-descendant tests and wrapper tests ---'
rg -n -C 5 'aria-activedescendant|SuggestionMenuWrapper|GridSuggestionMenuWrapper' \
--glob '*.{test,spec}.{ts,tsx,js,jsx}' \
--glob '!node_modules' .
printf '%s\n' '--- SuggestionMenuWrapper implementation ---'
cat -n packages/react/src/components/SuggestionMenu/SuggestionMenuWrapper.tsx
printf '%s\n' '--- GridSuggestionMenuWrapper implementation ---'
cat -n packages/react/src/components/SuggestionMenu/GridSuggestionMenu/GridSuggestionMenuWrapper.tsx
printf '%s\n' '--- test configuration and React test utilities ---'
rg -n -C 3 'vite-plus/test|vitest|testing-library|render\(' \
--glob '*.{json,js,jsx,ts,tsx,mjs,cjs}' \
packages/react package.json pnpm-workspace.yamlRepository: TypeCellOS/BlockNote
Length of output: 212
🏁 Script executed:
#!/bin/bash
printf '%s\n' '--- all active-descendant tests and wrapper tests ---'
rg -n -C 5 'aria-activedescendant|SuggestionMenuWrapper|GridSuggestionMenuWrapper' \
--glob '*.{test,spec}.{ts,tsx,js,jsx}' \
--glob '!node_modules' . || true
printf '%s\n' '--- SuggestionMenuWrapper implementation ---'
cat -n packages/react/src/components/SuggestionMenu/SuggestionMenuWrapper.tsx
printf '%s\n' '--- GridSuggestionMenuWrapper implementation ---'
cat -n packages/react/src/components/SuggestionMenu/GridSuggestionMenu/GridSuggestionMenuWrapper.tsx
printf '%s\n' '--- test configuration and React test utilities ---'
rg -n -C 3 'vite-plus/test|vitest|testing-library|render\(' \
--glob '*.{json,js,jsx,ts,tsx,mjs,cjs}' \
packages/react package.json pnpm-workspace.yaml || trueRepository: TypeCellOS/BlockNote
Length of output: 11649
🏁 Script executed:
#!/bin/bash
printf '%s\n' '--- keyboard navigation hooks ---'
cat -n packages/react/src/components/SuggestionMenu/hooks/useSuggestionMenuKeyboardNavigation.ts
cat -n packages/react/src/components/SuggestionMenu/GridSuggestionMenu/hooks/useGridSuggestionMenuKeyboardNavigation.ts
printf '%s\n' '--- package test dependencies and scripts ---'
rg -n -C 3 '"(test|react-testing-library|testing-library|jsdom|vite-plus)"|scripts:' \
packages/react/package.json package.json pnpm-workspace.yaml
printf '%s\n' '--- read-only coverage verifier ---'
python3 - <<'PY'
from pathlib import Path
import re
root = Path(".")
test_files = [
p for p in root.rglob("*")
if p.is_file()
and p.suffix in {".ts", ".tsx", ".js", ".jsx"}
and (".test." in p.name or ".spec." in p.name)
and "node_modules" not in p.parts
]
terms = {
"SuggestionMenuWrapper": re.compile(r"\bSuggestionMenuWrapper\b"),
"GridSuggestionMenuWrapper": re.compile(r"\bGridSuggestionMenuWrapper\b"),
"aria-activedescendant": re.compile(r"aria-activedescendant"),
}
for name, pattern in terms.items():
matches = []
for path in test_files:
text = path.read_text(errors="replace")
if pattern.search(text):
matches.append(str(path))
print(f"{name}: {len(matches)} matching test files")
for path in matches:
print(f" {path}")
PYRepository: TypeCellOS/BlockNote
Length of output: 6723
Add wrapper-level aria-activedescendant coverage
No test file covers either wrapper or this attribute. Add tests for index 0, a later index, undefined, and cleanup through both wrappers.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/react/src/components/SuggestionMenu/SuggestionMenu.test.tsx` around
lines 44 - 49, Extend the SuggestionMenu tests around getSuggestionMenuItemId to
cover wrapper-level aria-activedescendant behavior for both wrapper components.
Verify index 0, a later index, and undefined, and confirm the attribute is
removed during cleanup through each wrapper.
There was a problem hiding this comment.
Added direct wrapper coverage in 00624ef. The tests render both the list and grid wrappers, exercise index 0, a later index, and undefined, and verify unmount cleanup clears aria-activedescendant. The full @blocknote/react test and lint commands pass locally.
@blocknote/ariakit
@blocknote/code-block
@blocknote/core
@blocknote/mantine
@blocknote/react
@blocknote/server-util
@blocknote/shadcn
@blocknote/xl-ai
@blocknote/xl-docx-exporter
@blocknote/xl-email-exporter
@blocknote/xl-multi-column
@blocknote/xl-odt-exporter
@blocknote/xl-pdf-exporter
commit: |
|
Thanks for the PR @pupuking723 |
2 participants
Summary
Fix the suggestion menu's
aria-activedescendantvalue when the first item is selected.Fixes #2926.
Rationale
The wrappers previously used a truthiness check for
selectedIndex, so index0was treated like no selection and omitted from the accessibility attributes.Changes
Impact
This only changes the accessibility attribute for the existing first-item selection. Other selection behavior and the no-selection state are unchanged.
Testing
npx --yes pnpm@11.8.0 --filter @blocknote/react testnpx --yes pnpm@11.8.0 --filter @blocknote/react lintvp fmt --checkfor the changed filesgit diff --checkScreenshots/Video
Not applicable; this is an accessibility attribute fix with no visual change.
Checklist
Additional Notes
The regression test failed before the implementation because the helper was not yet available, and passes with the fix.
Summary by CodeRabbit
Accessibility
Tests