Skip to content

Commit

Permalink
Merge pull request #1861 from adevinta/fix-select-first-item
Browse files Browse the repository at this point in the history
fix(select): placeholder item for select
  • Loading branch information
Powerplex authored Feb 1, 2024
2 parents 153283d + f31e83c commit 62e35db
Show file tree
Hide file tree
Showing 8 changed files with 75 additions and 9 deletions.
1 change: 1 addition & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions packages/components/select/src/Select.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export const Default: StoryFn = _args => {
</Select.Trigger>

<Select.Items>
<Select.Placeholder>--Pick a book--</Select.Placeholder>
<Select.Item value="book-1">To Kill a Mockingbird</Select.Item>
<Select.Item value="book-2">War and Peace</Select.Item>
<Select.Item value="book-3">The Idiot</Select.Item>
Expand All @@ -49,6 +50,7 @@ export const Controlled: StoryFn = () => {
</Select.Trigger>

<Select.Items>
<Select.Placeholder>--Pick a book--</Select.Placeholder>
<Select.Item value="book-1">To Kill a Mockingbird</Select.Item>
<Select.Item value="book-2">War and Peace</Select.Item>
<Select.Item value="book-3">The Idiot</Select.Item>
Expand All @@ -70,6 +72,7 @@ export const Disabled: StoryFn = _args => {
</Select.Trigger>

<Select.Items>
<Select.Placeholder>--Pick a book--</Select.Placeholder>
<Select.Item value="book-1">To Kill a Mockingbird</Select.Item>
<Select.Item value="book-2">War and Peace</Select.Item>
<Select.Item value="book-3">The Idiot</Select.Item>
Expand All @@ -91,6 +94,7 @@ export const ReadOnly: StoryFn = _args => {
</Select.Trigger>

<Select.Items>
<Select.Placeholder>--Pick a book--</Select.Placeholder>
<Select.Item value="book-1">To Kill a Mockingbird</Select.Item>
<Select.Item value="book-2">War and Peace</Select.Item>
<Select.Item value="book-3">The Idiot</Select.Item>
Expand All @@ -112,6 +116,7 @@ export const DisabledItem: StoryFn = _args => {
</Select.Trigger>

<Select.Items>
<Select.Placeholder>--Pick a book--</Select.Placeholder>
<Select.Item value="book-1">To Kill a Mockingbird</Select.Item>
<Select.Item value="book-2">War and Peace</Select.Item>
<Select.Item value="book-3" disabled>
Expand All @@ -135,6 +140,7 @@ export const Grouped: StoryFn = _args => {
</Select.Trigger>

<Select.Items>
<Select.Placeholder>--Pick a book--</Select.Placeholder>
<Select.Group>
<Select.Label>Best-sellers</Select.Label>
<Select.Item value="book-1">To Kill a Mockingbird</Select.Item>
Expand Down Expand Up @@ -166,6 +172,7 @@ export const LeadingIcon: StoryFn = _args => {
</Select.Trigger>

<Select.Items>
<Select.Placeholder>--Pick a book--</Select.Placeholder>
<Select.Item value="book-1">To Kill a Mockingbird</Select.Item>
<Select.Item value="book-2">War and Peace</Select.Item>
<Select.Item value="book-3">The Idiot</Select.Item>
Expand Down
6 changes: 6 additions & 0 deletions packages/components/select/src/Select.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ describe('Select', () => {
</Select.Trigger>

<Select.Items>
<Select.Placeholder>--Pick a book--</Select.Placeholder>
<Select.Item value="book-1">War and Peace</Select.Item>
<Select.Item value="book-2">1984</Select.Item>
<Select.Item value="book-3">Pride and Prejudice</Select.Item>
Expand Down Expand Up @@ -135,6 +136,7 @@ describe('Select', () => {
</Select.Trigger>

<Select.Items>
<Select.Placeholder>--Pick a book--</Select.Placeholder>
<Select.Item value="book-1">{bookText}</Select.Item>
<Select.Item value="book-2">War and Peace</Select.Item>
<Select.Item value="book-3">The Idiot</Select.Item>
Expand Down Expand Up @@ -167,6 +169,7 @@ describe('Select', () => {
</Select.Trigger>

<Select.Items>
<Select.Placeholder>--Pick a book--</Select.Placeholder>
<Select.Item value="book-1">War and Peace</Select.Item>
<Select.Item value="book-2">1984</Select.Item>
<Select.Item value="book-3">Pride and Prejudice</Select.Item>
Expand Down Expand Up @@ -194,6 +197,7 @@ describe('Select', () => {
</Select.Trigger>

<Select.Items>
<Select.Placeholder>--Pick a book--</Select.Placeholder>
<Select.Item value="book-1">War and Peace</Select.Item>
<Select.Item value="book-2">1984</Select.Item>
<Select.Item value="book-3">Pride and Prejudice</Select.Item>
Expand Down Expand Up @@ -225,6 +229,7 @@ describe('Select', () => {
</Select.Trigger>

<Select.Items>
<Select.Placeholder>--Pick a book--</Select.Placeholder>
<Select.Item value="book-1">War and Peace</Select.Item>
<Select.Item value="book-2">1984</Select.Item>
<Select.Item value="book-3">Pride and Prejudice</Select.Item>
Expand All @@ -250,6 +255,7 @@ describe('Select', () => {
</Select.Trigger>

<Select.Items>
<Select.Placeholder>--Pick a book--</Select.Placeholder>
<Select.Item value="book-1">War and Peace</Select.Item>
<Select.Item value="book-2">1984</Select.Item>
<Select.Item value="book-3">Pride and Prejudice</Select.Item>
Expand Down
8 changes: 7 additions & 1 deletion packages/components/select/src/SelectContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ export interface SelectContextState {
fieldLabelId: string | undefined
name: string | undefined
required: boolean
placeholder: string | undefined
setPlaceholder: Dispatch<SetStateAction<string | undefined>>
}

export type SelectContextProps = PropsWithChildren<{
Expand Down Expand Up @@ -86,11 +88,13 @@ export const SelectProvider = ({
required: requiredProp,
}: SelectContextProps) => {
const [value, setValue] = useCombinedState(valueProp, defaultValue, onValueChange)
const [placeholder, setPlaceholder] = useState<string | undefined>(undefined)
const [itemsMap, setItemsMap] = useState<ItemsMap>(getItemsFromChildren(itemsComponent))
const [ariaLabel, setAriaLabel] = useState<string>()

// Computed state
const selectedItem = typeof value === 'string' ? itemsMap.get(value) : undefined
const firstItem = itemsMap.entries().next().value[1]
const selectedItem = typeof value === 'string' ? itemsMap.get(value) : firstItem
const isControlled = valueProp != null

// Derivated from FormField context
Expand Down Expand Up @@ -155,6 +159,8 @@ export const SelectProvider = ({
fieldLabelId,
name,
required,
placeholder,
setPlaceholder,
}}
>
{children}
Expand Down
32 changes: 32 additions & 0 deletions packages/components/select/src/SelectPlaceholder.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { forwardRef, type Ref, useEffect } from 'react'

import { useSelectContext } from './SelectContext'

export interface PlaceholderProps {
disabled?: boolean
children: string
}

export const Placeholder = forwardRef(
({ disabled = false, children }: PlaceholderProps, forwardedRef: Ref<HTMLOptionElement>) => {
const { setPlaceholder } = useSelectContext()

useEffect(() => {
setPlaceholder(children)
}, [children])

return (
<option
data-spark-component="select-placeholder"
ref={forwardedRef}
key="placeholder"
value=""
disabled={disabled}
>
{children}
</option>
)
}
)

Placeholder.displayName = 'Select.Placeholder'
21 changes: 14 additions & 7 deletions packages/components/select/src/SelectValue.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,22 @@ import { useSelectContext } from './SelectContext'
export interface ValueProps {
children?: ReactNode
className?: string
placeholder: string
/**
* Optional placeholder value for the trigger.
* If not specified, the value inside `Select.Placeholder` item will be used.
*/
placeholder?: string
}

export const Value = forwardRef(
({ children, className, placeholder }: ValueProps, forwardedRef: Ref<HTMLSpanElement>) => {
const { selectedItem } = useSelectContext()
(
{ children, className, placeholder: customPlaceholder }: ValueProps,
forwardedRef: Ref<HTMLSpanElement>
) => {
const { selectedItem, placeholder } = useSelectContext()

const hasSelectedItem = !!selectedItem
const text = selectedItem?.text
const isPlaceholderSelected = selectedItem?.value == null
const valuePlaceholder = customPlaceholder || placeholder

return (
<span
Expand All @@ -26,10 +33,10 @@ export const Value = forwardRef(
<span
className={cx(
'line-clamp-1 flex-1 overflow-hidden text-ellipsis break-all',
!hasSelectedItem && 'text-on-surface/dim-1'
isPlaceholderSelected && 'text-on-surface/dim-1'
)}
>
{!hasSelectedItem ? placeholder : children || text}
{isPlaceholderSelected ? valuePlaceholder : children || selectedItem?.text}
</span>
</span>
)
Expand Down
4 changes: 4 additions & 0 deletions packages/components/select/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { Item } from './SelectItem'
import { Items } from './SelectItems'
import { Label } from './SelectLabel'
import { LeadingIcon } from './SelectLeadingIcon'
import { Placeholder } from './SelectPlaceholder'
import { Trigger } from './SelectTrigger'
import { Value } from './SelectValue'

Expand All @@ -16,6 +17,7 @@ export const Select: FC<SelectProps> & {
Group: typeof Group
Item: typeof Item
Items: typeof Items
Placeholder: typeof Placeholder
Label: typeof Label
Trigger: typeof Trigger
Value: typeof Value
Expand All @@ -24,6 +26,7 @@ export const Select: FC<SelectProps> & {
Group,
Item,
Items,
Placeholder,
Label,
Trigger,
Value,
Expand All @@ -34,6 +37,7 @@ Select.displayName = 'Select'
Group.displayName = 'Select.Group'
Items.displayName = 'Select.Items'
Item.displayName = 'Select.Item'
Placeholder.displayName = 'Select.Placeholder'
Label.displayName = 'Select.Label'
Trigger.displayName = 'Select.Trigger'
Value.displayName = 'Select.Value'
Expand Down
5 changes: 4 additions & 1 deletion packages/components/select/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@ const getOrderedItems = (children: ReactNode, result: SelectItem[] = []): Select
React.Children.forEach(children, child => {
if (!isValidElement(child)) return

if (getElementDisplayName(child) === 'Select.Item') {
if (
getElementDisplayName(child) === 'Select.Item' ||
getElementDisplayName(child) === 'Select.Placeholder'
) {
const childProps = child.props as ItemProps
result.push({
value: childProps.value,
Expand Down

0 comments on commit 62e35db

Please sign in to comment.