Skip to content

Conversation

sergiocarracedo
Copy link
Collaborator

@sergiocarracedo sergiocarracedo commented Oct 7, 2025

Description

  • feat: ui select primitive component multiselection

Screenshots (if applicable)

Screen.Recording.2025-10-07.at.14.22.43.mov

[Link to Figma Design](Figma URL here)

Implementation details

We needed to rewrite the SelectPrimitive to allow multiselection: Implmenetation from: https://github.com/radix-ui/primitives/blob/b3ee588dcb339d6c6ce524fcdd968c5eeb4e8458/packages/react/select/src/select.tsx

@sergiocarracedo sergiocarracedo requested a review from a team as a code owner October 7, 2025 12:22
@Copilot Copilot AI review requested due to automatic review settings October 7, 2025 12:22
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces multiselect functionality to the Select primitive component by creating a custom Radix Select implementation with enhanced multiselection capabilities.

  • Extracts and modifies Radix UI Select primitive to support multiselection
  • Updates Select component to handle both single and multiple value selection
  • Refactors existing component dependencies to use the new custom primitive

Reviewed Changes

Copilot reviewed 20 out of 24 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
packages/react/src/ui/Select/components/radix-ui/select.tsx Creates custom Select primitive with multiselection support extracted from Radix UI
packages/react/src/ui/Select/components/radix-ui/index.ts Exports all components from custom Select primitive
packages/react/src/ui/Select/components/Select.tsx Updates main Select component to handle multiple values and use custom primitive
packages/react/src/ui/Select/components/SelectItem.tsx Adds checkbox support for multiselect items
packages/react/src/ui/Select/components/SelectContent.tsx Updates to use custom primitive and removes loading indicators
packages/react/src/ui/Select/SelectContext.tsx Enhances context to support multiple values
packages/react/src/ui/Select/stories/select.stories.tsx Adds multiselect story and updates imports
packages/react/package.json Adds new Radix UI dependencies for custom primitive
Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Contributor

github-actions bot commented Oct 7, 2025

🔍 Visual review for your branch is published 🔍

Here are the links to:

@sergiocarracedo sergiocarracedo marked this pull request as draft October 7, 2025 12:42
@sergiocarracedo sergiocarracedo force-pushed the feat/select-primitive-multiselect branch from e0034b3 to a7c6719 Compare October 7, 2025 13:32
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 20 out of 24 changed files in this pull request and generated 6 comments.

Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines 756 to 757
const isSelectedItem =
context.value !== undefined && context.value === value
Copy link

Copilot AI Oct 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comparison doesn't handle multiselect mode correctly. When context.value is an array (multiselect), this equality check will always be false. The logic should check if the value is included in the array when in multiselect mode.

Copilot uses AI. Check for mistakes.

Comment on lines 756 to 757
const isSelectedItem =
context.value !== undefined && context.value === value
Copy link

Copilot AI Oct 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue as above - this comparison in itemTextRefCallback doesn't handle multiselect arrays correctly and will prevent proper text selection highlighting in multiselect mode.

Copilot uses AI. Check for mistakes.

open: isOpen,
asList: props.asList,
multiple: props.multiple || false,
} as SelectContextType
Copy link

Copilot AI Oct 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid using type assertion (as SelectContextType). Instead, ensure the contextValue object properly satisfies the SelectContextType interface without requiring a type assertion.

Suggested change
} as SelectContextType
}

Copilot uses AI. Check for mistakes.

Copy link
Contributor

github-actions bot commented Oct 7, 2025

Coverage Report for packages/react

Status Category Percentage Covered / Total
🔵 Lines 17.58% 17369 / 98793
🔵 Statements 17.58% 17369 / 98793
🔵 Functions 44.1% 838 / 1900
🔵 Branches 70.72% 2438 / 3447
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
packages/react/src/experimental/Forms/Fields/Select/index.tsx 88.28% 71.76% 90% 88.28% 144-146, 226-228, 230-233, 238, 251, 284-288, 345-346, 414-431, 436-437, 444-446, 463-465, 467-468
packages/react/src/experimental/OneDataCollection/visualizations/collection/Card/index.tsx 78.54% 50% 77.77% 78.54% 149-150, 154-155, 161-162, 179-187, 191-199, 207, 219-221, 398-439
packages/react/src/experimental/OneDataCollection/visualizations/collection/List/index.tsx 7.38% 100% 0% 7.38% 57-265
packages/react/src/experimental/OneDataCollection/visualizations/collection/Table/Table.tsx 61.65% 80.32% 83.33% 61.65% 167-170, 260-275, 324-392, 403, 414-422, 437-506
packages/react/src/ui/GroupHeader/index.tsx 0% 0% 0% 0% 1
packages/react/src/ui/Select/Select.tsx 100% 100% 100% 100%
packages/react/src/ui/Select/SelectContext.tsx 100% 100% 100% 100%
packages/react/src/ui/Select/components/Select.tsx 89.04% 84.21% 100% 89.04% 83, 90-96
packages/react/src/ui/Select/components/SelectContent.tsx 92.14% 82.45% 75% 92.14% 141-143, 186-188, 232-238, 283-285
packages/react/src/ui/Select/components/SelectItem.tsx 86.04% 71.42% 0% 86.04% 24, 41-45
packages/react/src/ui/Select/components/SelectLabel.tsx 50% 100% 100% 50% 9-14
packages/react/src/ui/Select/components/SelectScrollButton.tsx 20.68% 100% 0% 20.68% 16-44
packages/react/src/ui/Select/components/SelectSeparator.tsx 100% 100% 100% 100%
packages/react/src/ui/Select/components/SelectTrigger.tsx 100% 50% 100% 100%
packages/react/src/ui/Select/components/radix-ui/index.ts 100% 100% 100% 100%
packages/react/src/ui/Select/components/radix-ui/select.tsx 65.75% 74.85% 75% 65.75% 234-238, 286-294, 298-310, 335-340, 345-366, 369-377, 401-434, 451-457, 508-519, 688-696, 699-709, 712-717, 749-750, 879-899, 944-949, 951-1079, 1107-1112, 1270-1299, 1326-1338, 1355-1360, 1401, 1414-1415, 1420-1425, 1433-1436, 1496, 1507-1510, 1513-1517, 1597-1598, 1624, 1644-1683, 1702-1744, 1757-1818, 1857-1868, 1888-1890, 1957-1958, 1983-1984
Generated in workflow #8464 for commit d09deab by the Vitest Coverage Report Action

Copy link
Contributor

github-actions bot commented Oct 7, 2025

size-limit report 📦

Path Size
JS: Stable 582.8 KB (+0.19% 🔺)
JS: Experimental 969.58 KB (+0.19% 🔺)
CSS 72.1 KB (+0.09% 🔺)

@sergiocarracedo sergiocarracedo marked this pull request as ready for review October 7, 2025 13:44
@sergiocarracedo sergiocarracedo force-pushed the feat/select-primitive-multiselect branch from a7c6719 to 4fc725c Compare October 7, 2025 13:48
@sergiocarracedo sergiocarracedo force-pushed the feat/select-primitive-multiselect branch from 4fc725c to bd2e8f8 Compare October 7, 2025 13:54
Comment on lines +180 to +199
"@radix-ui/number": "^1.1.1",
"@radix-ui/primitive": "^1.1.2",
"@radix-ui/react-collection": "^1.1.7",
"@radix-ui/react-compose-refs": "^1.1.2",
"@radix-ui/react-context": "^1.1.2",
"@radix-ui/react-direction": "^1.1.1",
"@radix-ui/react-dismissable-layer": "^1.1.10",
"@radix-ui/react-focus-guards": "^1.1.2",
"@radix-ui/react-focus-scope": "^1.1.7",
"@radix-ui/react-hover-card": "^1.1.15",
"@radix-ui/react-id": "^1.1.1",
"@radix-ui/react-popper": "^1.2.7",
"@radix-ui/react-portal": "^1.1.9",
"@radix-ui/react-primitive": "^2.1.3",
"@radix-ui/react-switch": "^1.2.6",
"@radix-ui/react-use-callback-ref": "^1.1.1",
"@radix-ui/react-use-controllable-state": "^1.2.2",
"@radix-ui/react-use-layout-effect": "^1.1.1",
"@radix-ui/react-use-previous": "^1.1.1",
"@radix-ui/react-visually-hidden": "^1.2.3",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to add all of this? 🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, to create the custom radix select primitive replacement

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants