Conversation
miriam-mr90
changed the title
✅ No New Circular DependenciesNo new circular dependencies detected. Current count: 0 |
📦 Alpha Package Version PublishedUse Use |
🔍 Visual review for your branch is published 🔍Here are the links to: |
Coverage Report for packages/react
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||
|
I promise that next time I’ll double-check all the proposed changes for a component beforehand and implement them all at once 🫠 |
There was a problem hiding this comment.
Pull request overview
This PR adds group-header as a new item type to the F0Select component's static options API, allowing consumers to visually separate options into labeled sections. The feature is fully backward compatible — existing item and separator types are unchanged.
Changes:
- Extended the
F0SelectItemPropsunion type with{ type: "group-header"; label: string }. - Updated
F0Select.tsxat five touch points (search filtering, selectability, value mapping,onChangeextraction, andgetItemsrendering) to handle the new type, with auto-separator insertion before non-first group headers. - Added a
WithGroupHeadersStorybook story and updatedWithSearchBox'ssearchFnto handle the new type.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
packages/react/src/components/F0Select/types.ts |
Adds { type: "group-header"; label: string } variant to the F0SelectItemProps union |
packages/react/src/components/F0Select/F0Select.tsx |
Implements rendering and exclusion logic for the new group-header type at all relevant call sites |
packages/react/src/components/F0Select/__stories__/F0Select.stories.tsx |
Adds WithGroupHeaders story and updates WithSearchBox custom searchFn |
You can also share your feedback on Copilot code review. Take the survey.
When a consumer places a manual separator before a group-header, the auto-separator logic would insert a second one. Now we check if the previous rendered item is already a separator (height === 1) and skip the auto-insertion in that case.
When search filtering removes all items under a group header, the header would still be visible. Now a post-processing pass strips group headers that have no selectable items between them and the next header or end of list, along with their auto-separators.
…leanup When search filtering removes all items, the splice operations in the orphaned group header cleanup could leave the array shorter than expected. Add a null check to prevent crash on undefined array access.
Cover group header rendering, accessibility (role=presentation), non-selectability (clicking header doesn't trigger onChange), orphaned header removal during search, and header restoration when search is cleared.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
You can also share your feedback on Copilot code review. Take the survey.
| export const WithGroupHeaders: Story = { | ||
| args: { | ||
| label: "Select a vacancy", | ||
| placeholder: "Select a vacancy", | ||
| showSearchBox: true, | ||
| value: undefined, | ||
| options: [ | ||
| { type: "group-header", label: "Assigned to me" }, | ||
| { value: "vacancy-1", label: "Senior Frontend Engineer" }, | ||
| { value: "vacancy-2", label: "Product Designer" }, | ||
| { type: "group-header", label: "All vacancies" }, | ||
| { value: "vacancy-3", label: "Backend Engineer" }, | ||
| { value: "vacancy-4", label: "Data Analyst" }, | ||
| { value: "vacancy-5", label: "DevOps Engineer" }, | ||
| { value: "vacancy-6", label: "QA Engineer" }, | ||
| ], | ||
| }, | ||
| } |
There was a problem hiding this comment.
The Snapshot story is used for Chromatic visual regression testing, but the new WithGroupHeaders variant is not included in it. Per the F0 code review checklist (SKILL.md), all meaningful visual variants should be represented in the Snapshot story. Consider adding a group-headers variant to the Snapshot story so the group-header styling is captured in visual regression tests.
Description
Add
group-headeras a new item type for theF0Selectcomponent when using staticoptions. This allows consumers to visually separate options into labeled sections (e.g. "Assigned to me" / "All vacancies") without resorting to disabled items or manual separators.A
SelectSeparatoris automatically rendered before each group header (except the first), so consumers only need to insert{ type: "group-header", label: "..." }entries in their options array.Fully backward compatible — existing
itemandseparatortypes continue to work unchanged.Screenshots
The dropdown shows two group headers ("Assigned to me" and "All vacancies") separating selectable options, with an automatic separator line between groups.
Implementation details
types.ts: ExtendedF0SelectItemPropsunion with a new{ type: "group-header"; label: string }variant alongside the existingitemandseparatortypes.F0Select.tsx(5 touch points):defaultSearchFn— skipsgroup-headeritems from search filtering (same asseparator)selectablecallback — excludesgroup-headerfrom selectable itemsitemsByValuemapping — excludesgroup-headerfrom the value maponChangehandler — excludesgroup-headerfrom value extractiongetItemscallback — renders a styleddivmatching the Figma "Dropdown header" spec (secondary text,font-medium,text-sm, padding 12/8/12px) with autoSelectSeparatorbefore non-first group headersF0Select.stories.tsx: Added aWithGroupHeadersstory demonstrating group headers with static options. Updated theWithSearchBoxstory's customsearchFnto handle the new type.Verification
Tests
Manual Verification
WithGroupHeadersstory