Add grouped suggestions#222
Conversation
Key changes: Added MentionDataSection and MentionDataPage.sections support in types.ts. Normalized grouped pages into flat selectable items plus section metadata. Preserved flat keyboard/focus indexes while rendering non-focusable section headers in SuggestionsOverlay.tsx. Added pagination section merging by stable section identity. Added class slots: suggestionSection, suggestionSectionLabel. Added tests, README docs, and a new demo: GroupedSuggestions.tsx.
📝 WalkthroughWalkthroughThis PR adds support for grouped/sectioned mention suggestions. It introduces new types for section-based mention data, updates rendering and state management to display section headers while treating sections as non-focusable, and extends helper utilities to flatten and normalize sectioned results alongside pagination merging. Documentation and demo examples accompany the changes. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant MentionsInput
participant Provider as Async Provider
participant Selectors as Normalization
participant Overlay as SuggestionsOverlay
participant Handler as Selection Handler
User->>MentionsInput: Type `@query`
MentionsInput->>Provider: fetchGroupedDirectory(query)
Provider-->>MentionsInput: { sections: [{ label, items }] }
MentionsInput->>Selectors: normalizeMentionDataResult(page)
Selectors-->>MentionsInput: { items: [...flattened], sections: [...normalized] }
MentionsInput->>Overlay: Pass suggestions with sections
Overlay->>Overlay: flattenSuggestionRenderEntries(suggestions)
Overlay-->>Overlay: [SectionHeader, Item0, Item1, SectionHeader, Item2]
Overlay->>User: Render section labels (non-focusable)<br/>+ selectable items
User->>Overlay: ArrowDown to Item1
Overlay->>Handler: onMouseEnter(suggestionIndex: 1)
User->>Overlay: Enter to select Item1
Overlay->>Handler: onSelect(item, queryInfo)
Handler->>MentionsInput: Update mention
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #222 +/- ##
==========================================
- Coverage 97.90% 97.45% -0.45%
==========================================
Files 58 58
Lines 2191 2279 +88
Branches 568 599 +31
==========================================
+ Hits 2145 2221 +76
- Misses 10 15 +5
- Partials 36 43 +7
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Code Review
This pull request introduces support for grouped suggestion sections, allowing data providers to return categorized lists (e.g., Users and Teams) under a single trigger. The implementation includes updates to the internal state management to handle section merging during pagination, a new flattening utility to interleave non-selectable section headers with selectable items, and enhanced DOM selection logic to ensure keyboard navigation remains accurate despite the non-flat list structure. Additionally, new styling properties for sections have been added to the component interface, accompanied by comprehensive unit and integration tests. I have no feedback to provide.
|
Check compatibility with signavio/react-mentions#787 |
There was a problem hiding this comment.
Pull request overview
Adds first-class support for grouped suggestions (section headers + items) across the provider API, normalization/query state, overlay rendering, demo, and documentation.
Changes:
- Extend provider result types to allow returning
sections(Users/Teams-style grouping) and carry section metadata through suggestion state. - Render section headers in
SuggestionsOverlaywhile keeping keyboard navigation and selectable indexes counting only actual suggestion items. - Add/adjust tests, demo example, and docs; bump Vitest/Knip dev deps.
Reviewed changes
Copilot reviewed 20 out of 21 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/MentionsInput.spec.tsx | Adds an integration test for grouped async suggestions and keyboard selection behavior. |
| src/utils/index.ts | Re-exports flattenSuggestionRenderEntries for overlay rendering. |
| src/utils/flattenSuggestions.ts | Adds section-aware “render entries” flattening (headers + items) while preserving selectable item indexing. |
| src/utils/flattenSuggestions.spec.tsx | Tests section header emission and index sequencing. |
| src/useSuggestionsQuery.ts | Preserves sections through loading/preserved suggestion states. |
| src/types.ts | Introduces MentionDataSection / SuggestionSection and updates MentionDataPage / suggestion state types for grouping. |
| src/index.ts | Exports the new public MentionDataSection type. |
| src/SuggestionsOverlay.tsx | Renders section headers and updates scroll-to-focused logic to avoid counting headers as options. |
| src/SuggestionsOverlay.spec.tsx | Tests grouped section header rendering and index stability. |
| src/Suggestion.tsx | Adds data-suggestion-index attribute for testing/diagnostics. |
| src/MentionsInputSelectors.ts | Normalizes provider sections into flat items + section metadata and includes sections in layout keys. |
| src/MentionsInputSelectors.spec.tsx | Updates layout-key expectations and tests section normalization. |
| src/MentionsInputQueryState.ts | Stores/merges section metadata across query + pagination results. |
| src/MentionsInputQueryState.spec.ts | Adds tests for section persistence and section merging across pagination. |
| src/MentionsInput.tsx | Wires new section classNames through to the overlay. |
| pnpm-lock.yaml | Updates lockfile for bumped dev dependencies. |
| package.json | Bumps Vitest-related packages and Knip. |
| demo/src/examples/mentionsClassNames.ts | Adds section-related classNames for the demo styles. |
| demo/src/examples/GroupedSuggestions.tsx | New demo example showing grouped suggestions provider usage. |
| demo/src/examples/Examples.tsx | Registers the new grouped suggestions demo card. |
| README.md | Documents grouped suggestions usage and adds it to the demo/features list. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const previousResults = Object.hasOwn(currentSuggestions, childIndex) | ||
| ? currentSuggestions[childIndex].results | ||
| : [] | ||
| const previousSections = Object.hasOwn(currentSuggestions, childIndex) | ||
| ? currentSuggestions[childIndex].sections | ||
| : undefined | ||
| const results = [...previousResults, ...page.items] | ||
| const sections = mergeSuggestionSections(previousSections, page.sections) | ||
| const suggestions: SuggestionsMap<Extra> = { | ||
| ...currentSuggestions, | ||
| [childIndex]: { | ||
| queryInfo, | ||
| results, | ||
| }, | ||
| [childIndex]: createSuggestionsEntry(queryInfo, results, sections), | ||
| } |
| | (MentionDataPageBase & { | ||
| items: ReadonlyArray<MentionDataItem<Extra>> | ||
| sections?: never | ||
| }) | ||
| | (MentionDataPageBase & { | ||
| items?: never | ||
| sections: ReadonlyArray<MentionDataSection<Extra>> | ||
| }) |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/SuggestionsOverlay.tsx (1)
273-279: 🧹 Nitpick | 🔵 TrivialAvoid flattening the suggestion tree twice in this hot path.
flattenSuggestionsandflattenSuggestionRenderEntriesboth walk the same suggestion map, but the first result is only used forlength > 0. Derive that boolean from the render entries instead so overlay rendering stays to a single traversal.As per coding guidelines "Preserve render locality: move state and derived work into the smallest branch that consumes it, and do not reparse mention children/config in hot render paths".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/SuggestionsOverlay.tsx` around lines 273 - 279, The code currently calls flattenSuggestions and flattenSuggestionRenderEntries, doing two traversals; remove the flattenSuggestions use in this hot path and derive the "has suggestions" boolean from flattenedSuggestionRenderEntries instead: eliminate the useMemo that computes flattenedSuggestions (and any references to flattenedSuggestions.length > 0), update the overlay rendering logic to use flattenedSuggestionRenderEntries (from flattenSuggestionRenderEntries(mentionChildren, suggestions)) to determine emptiness and render accordingly, and keep flattenSuggestions only if elsewhere referenced; adjust any variable names/usages so only one traversal over mentionChildren/suggestions occurs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/MentionsInputSelectors.spec.tsx`:
- Around line 711-717: The test constructs a MentionDataSection object named
publicSection with an unused items property which trips
unicorn/no-unused-properties; either remove the unused items key when creating
publicSection or use it in an assertion—e.g., construct publicSection = { label:
'Public API section' } only, or add an assertion against publicSection.items
(like expect(publicSection.items).toEqual([])) so the items property is
referenced; update the test around the publicSection declaration/assertions
accordingly.
In `@src/MentionsInputSelectors.ts`:
- Around line 94-100: The fallback key for sections that lack `id` can collide
when two sections share the same string `label`; update the label branch in
MentionsInputSelectors to include a disambiguator (e.g., append the
`sectionIndex` or a stable hash) so keys become unique per-section: when
`section.id` is undefined, return a string that includes `section.label` plus
`sectionIndex` (or another stable unique token) instead of only
`label:${section.label}`; ensure you use `String(sectionIndex)` or similar to
produce a stable string key so downstream code in flattenSuggestions and
MentionsInputQueryState no longer sees duplicate keys.
In `@src/utils/flattenSuggestions.ts`:
- Around line 91-146: The flattenSuggestionRenderEntries function is too complex
for the linter; extract the inner "process sections" and "process unsectioned
results" logic into two small helpers (e.g., pushSectionEntries and
pushUnsectionedResults) to reduce cognitive complexity. Each helper should
accept the flattened array, the entry/queryInfo, the childIndex or sectionKey as
needed, the results or section.results, the shared handledResults Set, and the
current suggestionIndex and return the updated suggestionIndex (or mutate a
passed object) so suggestionIndex is preserved; ensure keys use the same formats
(sectionKey-item-... and childIndex-item-...), keep use of
getSuggestionId(result), and keep the original types
(FlattenedSuggestionRenderEntry, SuggestionDataItem, SuggestionsMap) so behavior
and types remain unchanged.
---
Outside diff comments:
In `@src/SuggestionsOverlay.tsx`:
- Around line 273-279: The code currently calls flattenSuggestions and
flattenSuggestionRenderEntries, doing two traversals; remove the
flattenSuggestions use in this hot path and derive the "has suggestions" boolean
from flattenedSuggestionRenderEntries instead: eliminate the useMemo that
computes flattenedSuggestions (and any references to flattenedSuggestions.length
> 0), update the overlay rendering logic to use flattenedSuggestionRenderEntries
(from flattenSuggestionRenderEntries(mentionChildren, suggestions)) to determine
emptiness and render accordingly, and keep flattenSuggestions only if elsewhere
referenced; adjust any variable names/usages so only one traversal over
mentionChildren/suggestions occurs.
🪄 Autofix (Beta)
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5c545249-68e0-4029-b529-623047b4aec5
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (20)
README.mddemo/src/examples/Examples.tsxdemo/src/examples/GroupedSuggestions.tsxdemo/src/examples/mentionsClassNames.tspackage.jsonsrc/MentionsInput.tsxsrc/MentionsInputQueryState.spec.tssrc/MentionsInputQueryState.tssrc/MentionsInputSelectors.spec.tsxsrc/MentionsInputSelectors.tssrc/Suggestion.tsxsrc/SuggestionsOverlay.spec.tsxsrc/SuggestionsOverlay.tsxsrc/index.tssrc/types.tssrc/useSuggestionsQuery.tssrc/utils/flattenSuggestions.spec.tsxsrc/utils/flattenSuggestions.tssrc/utils/index.tstests/MentionsInput.spec.tsx
| const publicSection: MentionDataSection = { | ||
| label: 'Public API section', | ||
| items: [], | ||
| } | ||
|
|
||
| expect(publicSection.label).toBe('Public API section') | ||
|
|
There was a problem hiding this comment.
Use publicSection.items or avoid constructing it.
Only label is asserted here, so items stays unused and trips unicorn/no-unused-properties. This will keep the test file failing lint.
Minimal fix
- expect(publicSection.label).toBe('Public API section')
+ expect(publicSection).toEqual({
+ label: 'Public API section',
+ items: [],
+ })📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const publicSection: MentionDataSection = { | |
| label: 'Public API section', | |
| items: [], | |
| } | |
| expect(publicSection.label).toBe('Public API section') | |
| const publicSection: MentionDataSection = { | |
| label: 'Public API section', | |
| items: [], | |
| } | |
| expect(publicSection).toEqual({ | |
| label: 'Public API section', | |
| items: [], | |
| }) | |
🧰 Tools
🪛 ESLint
[error] 713-713: Property items is defined but never used.
(unicorn/no-unused-properties)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/MentionsInputSelectors.spec.tsx` around lines 711 - 717, The test
constructs a MentionDataSection object named publicSection with an unused items
property which trips unicorn/no-unused-properties; either remove the unused
items key when creating publicSection or use it in an assertion—e.g., construct
publicSection = { label: 'Public API section' } only, or add an assertion
against publicSection.items (like expect(publicSection.items).toEqual([])) so
the items property is referenced; update the test around the publicSection
declaration/assertions accordingly.
| if (section.id !== undefined) { | ||
| return `id:${typeof section.id}:${String(section.id)}` | ||
| } | ||
|
|
||
| return typeof section.label === 'string' | ||
| ? `label:${section.label}` | ||
| : `index:${sectionIndex.toString()}` |
There was a problem hiding this comment.
Disambiguate fallback section keys.
Two sections that omit id but share the same string label both get the same key here. That gives src/utils/flattenSuggestions.ts duplicate React keys and makes src/MentionsInputQueryState.ts merge unrelated sections together during pagination. Include a disambiguator in the label fallback at minimum.
Possible fix
if (section.id !== undefined) {
return `id:${typeof section.id}:${String(section.id)}`
}
return typeof section.label === 'string'
- ? `label:${section.label}`
+ ? `label:${section.label}:${sectionIndex.toString()}`
: `index:${sectionIndex.toString()}`As per coding guidelines "Never define components inside components; preserve stable keys and element identity across suggestions, highlighter output, and other dynamic lists".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (section.id !== undefined) { | |
| return `id:${typeof section.id}:${String(section.id)}` | |
| } | |
| return typeof section.label === 'string' | |
| ? `label:${section.label}` | |
| : `index:${sectionIndex.toString()}` | |
| if (section.id !== undefined) { | |
| return `id:${typeof section.id}:${String(section.id)}` | |
| } | |
| return typeof section.label === 'string' | |
| ? `label:${section.label}:${sectionIndex.toString()}` | |
| : `index:${sectionIndex.toString()}` |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/MentionsInputSelectors.ts` around lines 94 - 100, The fallback key for
sections that lack `id` can collide when two sections share the same string
`label`; update the label branch in MentionsInputSelectors to include a
disambiguator (e.g., append the `sectionIndex` or a stable hash) so keys become
unique per-section: when `section.id` is undefined, return a string that
includes `section.label` plus `sectionIndex` (or another stable unique token)
instead of only `label:${section.label}`; ensure you use `String(sectionIndex)`
or similar to produce a stable string key so downstream code in
flattenSuggestions and MentionsInputQueryState no longer sees duplicate keys.
| export const flattenSuggestionRenderEntries = < | ||
| Extra extends Record<string, unknown> = Record<string, unknown>, | ||
| >( | ||
| children: ReactNode, | ||
| suggestions: SuggestionsMap<Extra> | undefined | ||
| ): Array<FlattenedSuggestionRenderEntry<Extra>> => { | ||
| const flattened: Array<FlattenedSuggestionRenderEntry<Extra>> = [] | ||
| let suggestionIndex = 0 | ||
|
|
||
| for (const [childIndex, entry] of getOrderedSuggestionMapEntries(children, suggestions)) { | ||
| const { queryInfo, results, sections } = entry | ||
| const handledResults = new Set<SuggestionDataItem<Extra>>() | ||
|
|
||
| if (sections !== undefined && sections.length > 0) { | ||
| for (const section of sections) { | ||
| const sectionKey = `${childIndex.toString()}-section-${section.key}` | ||
| flattened.push({ | ||
| type: 'section', | ||
| key: sectionKey, | ||
| label: section.label, | ||
| queryInfo, | ||
| }) | ||
|
|
||
| for (const [sectionResultIndex, result] of section.results.entries()) { | ||
| handledResults.add(result) | ||
| flattened.push({ | ||
| type: 'item', | ||
| key: `${sectionKey}-item-${getSuggestionId(result)}-${sectionResultIndex.toString()}`, | ||
| index: suggestionIndex, | ||
| result, | ||
| queryInfo, | ||
| sectionKey, | ||
| }) | ||
| suggestionIndex += 1 | ||
| } | ||
| } | ||
| } | ||
|
|
||
| for (const [resultIndex, result] of results.entries()) { | ||
| if (handledResults.has(result)) { | ||
| continue | ||
| } | ||
|
|
||
| flattened.push({ | ||
| type: 'item', | ||
| key: `${childIndex.toString()}-item-${getSuggestionId(result)}-${resultIndex.toString()}`, | ||
| index: suggestionIndex, | ||
| result, | ||
| queryInfo, | ||
| }) | ||
| suggestionIndex += 1 | ||
| } | ||
| } | ||
|
|
||
| return flattened | ||
| } |
There was a problem hiding this comment.
Split flattenSuggestionRenderEntries into smaller helpers so lint passes.
This now exceeds the repo's sonarjs/cognitive-complexity limit, so the changed file will fail lint. Extract the section pass and the trailing unsectioned-results pass into small helpers.
🧰 Tools
🪛 ESLint
[error] 96-96: Refactor this function to reduce its Cognitive Complexity from 16 to the 15 allowed.
(sonarjs/cognitive-complexity)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/utils/flattenSuggestions.ts` around lines 91 - 146, The
flattenSuggestionRenderEntries function is too complex for the linter; extract
the inner "process sections" and "process unsectioned results" logic into two
small helpers (e.g., pushSectionEntries and pushUnsectionedResults) to reduce
cognitive complexity. Each helper should accept the flattened array, the
entry/queryInfo, the childIndex or sectionKey as needed, the results or
section.results, the shared handledResults Set, and the current suggestionIndex
and return the updated suggestionIndex (or mutate a passed object) so
suggestionIndex is preserved; ensure keys use the same formats
(sectionKey-item-... and childIndex-item-...), keep use of
getSuggestionId(result), and keep the original types
(FlattenedSuggestionRenderEntry, SuggestionDataItem, SuggestionsMap) so behavior
and types remain unchanged.
There was a problem hiding this comment.
3 issues found across 21 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/SuggestionsOverlay.tsx">
<violation number="1" location="src/SuggestionsOverlay.tsx:112">
P2: Expose grouped suggestion labels with listbox semantics instead of rendering them as presentational items.</violation>
</file>
<file name="src/MentionsInputQueryState.ts">
<violation number="1" location="src/MentionsInputQueryState.ts:135">
P2: Merging an existing section key keeps the old label/id, so paginated section headers can go stale.</violation>
</file>
<file name="src/utils/flattenSuggestions.ts">
<violation number="1" location="src/utils/flattenSuggestions.ts:105">
P1: Rendering all sectioned items before leftover results can desync the visible order from keyboard focus order.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| const handledResults = new Set<SuggestionDataItem<Extra>>() | ||
|
|
||
| if (sections !== undefined && sections.length > 0) { | ||
| for (const section of sections) { |
There was a problem hiding this comment.
P1: Rendering all sectioned items before leftover results can desync the visible order from keyboard focus order.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/utils/flattenSuggestions.ts, line 105:
<comment>Rendering all sectioned items before leftover results can desync the visible order from keyboard focus order.</comment>
<file context>
@@ -49,4 +88,61 @@ const flattenSuggestions = <Extra extends Record<string, unknown> = Record<strin
+ const handledResults = new Set<SuggestionDataItem<Extra>>()
+
+ if (sections !== undefined && sections.length > 0) {
+ for (const section of sections) {
+ const sectionKey = `${childIndex.toString()}-section-${section.key}`
+ flattened.push({
</file context>
| className, | ||
| labelClassName, | ||
| }: SuggestionSectionHeaderProps) => ( | ||
| <li role="presentation" className={cn(sectionStyles(), className)} data-slot="suggestion-section"> |
There was a problem hiding this comment.
P2: Expose grouped suggestion labels with listbox semantics instead of rendering them as presentational items.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/SuggestionsOverlay.tsx, line 112:
<comment>Expose grouped suggestion labels with listbox semantics instead of rendering them as presentational items.</comment>
<file context>
@@ -89,6 +98,24 @@ const statusStyles = cva('px-4 py-2.5 text-left text-sm leading-relaxed', {
+ className,
+ labelClassName,
+}: SuggestionSectionHeaderProps) => (
+ <li role="presentation" className={cn(sectionStyles(), className)} data-slot="suggestion-section">
+ <span className={cn(sectionLabelStyles(), labelClassName)} data-slot="suggestion-section-label">
+ {label}
</file context>
| ...existingSection, | ||
| results: [...existingSection.results, ...section.results], |
There was a problem hiding this comment.
P2: Merging an existing section key keeps the old label/id, so paginated section headers can go stale.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/MentionsInputQueryState.ts, line 135:
<comment>Merging an existing section key keeps the old label/id, so paginated section headers can go stale.</comment>
<file context>
@@ -82,6 +83,63 @@ const clampFocusIndex = <Extra extends Record<string, unknown>>(
+
+ const existingSection = mergedSections[existingSectionIndex]
+ mergedSections[existingSectionIndex] = {
+ ...existingSection,
+ results: [...existingSection.results, ...section.results],
+ }
</file context>
| ...existingSection, | |
| results: [...existingSection.results, ...section.results], | |
| ...section, | |
| results: [...existingSection.results, ...section.results], |
Summary by CodeRabbit
Release Notes
New Features
Documentation
Chores
Note
Add grouped suggestions support to MentionsInput
MentionDataPagediscriminated union type in types.ts.SuggestionsOverlayrenders non-focusable section headers between items using the newflattenSuggestionRenderEntriesutility; keyboard navigation and selection index only the actual items.suggestionSection,suggestionSectionLabel) are added toMentionsInputClassNamesand passed through fromMentionsInputto the overlay for styling.getSuggestionsLayoutKeynow incorporates section structure, which may trigger additional overlay rerenders when sections change even if items are unchanged.Macroscope summarized 341b750.