Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adjust Select selected options UI #1659

Merged
merged 2 commits into from
Aug 19, 2024
Merged

Adjust Select selected options UI #1659

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
84d7b54
Adjust Select selected options UI
acelaya Aug 19, 2024
a02ae4e
Adjust Select examples to look good with new UI
acelaya Aug 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 19 additions & 8 deletions src/components/input/Select.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { useSyncedRef } from '../../hooks/use-synced-ref';
import type { CompositeProps } from '../../types';
import { ListenerCollection } from '../../util/listener-collection';
import { downcastRef } from '../../util/typing';
import { MenuCollapseIcon, MenuExpandIcon } from '../icons';
import { CheckIcon, MenuCollapseIcon, MenuExpandIcon } from '../icons';
import { inputGroupStyles } from './InputGroup';
import SelectContext from './SelectContext';

Expand Down Expand Up @@ -99,11 +99,10 @@ function SelectOption<T>({
<li
className={classnames(
'w-full ring-inset outline-none rounded-none select-none',
'border-t first:border-t-0 whitespace-nowrap',
'px-1 mb-1 first:mt-1 whitespace-nowrap group',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have decided to implement spacing using margins and padding on the options, so that it's easier to eventually include the search boxes.

Copy link
Member

Choose a reason for hiding this comment

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

This is fine, but can you clarify how this makes adding the search boxes easier?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes.

I initially considered adding a small padding to the surrounding listbox, but according to the designs, the search box container should span to the very sides of said listbox.

image

Adding the padding to every individual option instead, ensures that will be eventually possible.

{
'text-grey-4': disabled,
'cursor-pointer focus-visible-ring hover:bg-grey-1': !disabled,
'bg-grey-1 hover:bg-grey-2': selected,
'cursor-pointer': !disabled,
},
classes,
)}
Expand All @@ -125,12 +124,24 @@ function SelectOption<T>({
tabIndex={-1}
>
<div
className={classnames('flex w-full p-1.5 border-l-4', {
'border-l-transparent': !selected,
'border-l-brand font-medium': selected,
})}
className={classnames(
'w-full flex justify-between items-center gap-2',
'rounded py-2 px-3',
{
'hover:bg-grey-1 group-focus-visible:ring': !disabled,
'bg-grey-1 hover:bg-grey-2': selected,
},
)}
>
{optionChildren(children, { selected, disabled })}
<CheckIcon
className={classnames('text-grey-6 scale-125', {
// Make the icon visible/invisible, instead of conditionally
// rendering it, to ensure consistent spacing among selected and
// non-selected options
'opacity-0': !selected,
})}
/>
</div>
</li>
);
Expand Down
13 changes: 6 additions & 7 deletions src/pattern-library/components/patterns/prototype/SelectPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,10 @@ function SelectExample({
{textOnly && value.name}
{!textOnly && (
<div className="flex">
<div className="truncate">{value.name}</div>
<div className="rounded px-2 ml-2 bg-grey-7 text-white">
<div className="rounded px-2 mr-2 bg-grey-7 text-white">
{value.id}
</div>
<div className="truncate">{value.name}</div>
</div>
)}
</>
Expand All @@ -68,18 +68,17 @@ function SelectExample({
textOnly ? (
item.name
) : (
<>
{item.name}
<div className="grow" />
<div className="flex">
<div
className={classnames('rounded px-2 ml-2 text-white', {
className={classnames('rounded px-2 mr-2 text-white', {
'bg-grey-7': !disabled,
'bg-grey-4': disabled,
})}
>
{item.id}
</div>
</>
<div className="truncate">{item.name}</div>
</div>
)
}
</Select.Option>
Expand Down
29 changes: 14 additions & 15 deletions src/pattern-library/examples/select-in-input-group.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import classnames from 'classnames';
import { useCallback, useId, useMemo, useState } from 'preact/hooks';

import { ArrowLeftIcon, ArrowRightIcon } from '../../components/icons';
Expand All @@ -13,14 +12,25 @@ const students = [
{ id: '5', name: 'Doris Evanescence' },
];

type Student = (typeof students)[number];

function StudentContainer({ student }: { student: Student }) {
return (
<div className="flex">
<div className="rounded px-2 mr-2 bg-grey-7 text-white">{student.id}</div>
<div className="truncate">{student.name}</div>
</div>
);
}

export default function App({
buttonClasses,
wrapperClasses = 'w-96',
}: {
buttonClasses?: string;
wrapperClasses?: string;
}) {
const [selected, setSelected] = useState<(typeof students)[number]>();
const [selected, setSelected] = useState<Student>();
const selectedIndex = useMemo(
() => (!selected ? -1 : students.findIndex(item => item === selected)),
[selected],
Expand Down Expand Up @@ -53,26 +63,15 @@ export default function App({
buttonClasses={buttonClasses}
buttonContent={
selected ? (
<div className="flex">
<div className="truncate">{selected.name}</div>
<div className="rounded px-2 ml-2 bg-grey-7 text-white">
{selected.id}
</div>
</div>
<StudentContainer student={selected} />
) : (
<>Select one…</>
)
}
>
{students.map(item => (
<Select.Option value={item} key={item.id}>
{item.name}
<div className="grow" />
<div
className={classnames('rounded px-2 ml-2 text-white bg-grey-7')}
>
{item.id}
</div>
<StudentContainer student={item} />
</Select.Option>
))}
</Select>
Expand Down
19 changes: 1 addition & 18 deletions src/pattern-library/examples/select-non-popover-listbox.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,20 +34,7 @@ export default function App() {
buttonId={buttonId}
value={value}
onChange={setValue}
buttonContent={
value ? (
<>
<div className="flex">
<div className="truncate">{value.name}</div>
<div className="rounded px-2 ml-2 bg-grey-7 text-white">
{value.id}
</div>
</div>
</>
) : (
<>Select one…</>
)
}
buttonContent={value ? value.name : <>Select one…</>}
>
{items.map(item => (
<Select.Option
Expand All @@ -56,10 +43,6 @@ export default function App() {
disabled={item.disabled}
>
{item.name}
<div className="grow" />
<div className="rounded px-2 ml-2 text-white bg-grey-7">
{item.id}
</div>
</Select.Option>
))}
</Select>
Expand Down
25 changes: 10 additions & 15 deletions src/pattern-library/examples/select-right.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import type { ComponentChildren } from 'preact';
import { useId, useState } from 'preact/hooks';

import { Select } from '../..';
Expand All @@ -11,14 +10,19 @@ const items = [
{ id: '5', name: 'Doris Evanescence' },
];

function Bullet({ children }: { children: ComponentChildren }) {
type Item = (typeof items)[number];

function ItemContainer({ item }: { item: Item }) {
return (
<div className="rounded px-2 ml-2 bg-grey-7 text-white">{children}</div>
<div className="flex">
<div className="rounded px-2 mr-2 bg-grey-7 text-white">{item.id}</div>
<div className="truncate">{item.name}</div>
</div>
);
}

export default function App() {
const [value, setSelected] = useState<{ id: string; name: string }>();
const [value, setSelected] = useState<Item>();
const selectId = useId();

return (
Expand All @@ -30,22 +34,13 @@ export default function App() {
onChange={setSelected}
buttonId={selectId}
buttonContent={
value ? (
<div className="flex">
<div className="truncate">{value.name}</div>
<Bullet>{value.id}</Bullet>
</div>
) : (
<>Select one…</>
)
value ? <ItemContainer item={value} /> : <>Select one…</>
}
buttonClasses="!w-36"
>
{items.map(item => (
<Select.Option value={item} key={item.id}>
{item.name}
<div className="grow" />
<Bullet>{item.id}</Bullet>
<ItemContainer item={item} />
</Select.Option>
))}
</Select>
Expand Down
21 changes: 7 additions & 14 deletions src/pattern-library/examples/select-with-custom-options.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import type { ComponentChildren } from 'preact';
import { useId, useState } from 'preact/hooks';

import { Select } from '../..';
Expand All @@ -16,9 +15,12 @@ const defaultItems: Item[] = [
{ id: '5', name: 'Doris Evanescence' },
];

function Bullet({ children }: { children: ComponentChildren }) {
function ItemContainer({ item }: { item: Item }) {
return (
<div className="rounded px-2 ml-2 bg-grey-7 text-white">{children}</div>
<div className="flex">
<div className="rounded px-2 mr-2 bg-grey-7 text-white">{item.id}</div>
<div className="truncate">{item.name}</div>
</div>
);
}

Expand All @@ -34,21 +36,12 @@ export default function App({ items = defaultItems }: { items?: Item[] }) {
onChange={setSelected}
buttonId={selectId}
buttonContent={
value ? (
<div className="flex">
<div className="truncate">{value.name}</div>
<Bullet>{value.id}</Bullet>
</div>
) : (
<>Select one…</>
)
value ? <ItemContainer item={value} /> : <>Select one…</>
}
>
{items.map(item => (
<Select.Option value={item} key={item.id}>
{item.name}
<div className="grow" />
<Bullet>{item.id}</Bullet>
<ItemContainer item={item} />
</Select.Option>
))}
</Select>
Expand Down