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.

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

move tag-container outside select-button #2346

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions .changeset/fresh-numbers-yawn-1.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@itwin/itwinui-react': patch
---

`<Select multiple>` now renders the tag container outside the select-button. While this shouldn't make a noticeable difference in most cases, it might affect the use of `selectedItemRenderer`.
29 changes: 29 additions & 0 deletions .changeset/fresh-numbers-yawn.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
---
'@itwin/itwinui-css': major
---

Multi-select with now requires the tag-container to be rendered outside the select-button, and also requires an additional `data-iui-multi` attribute on the select-button.

<details>
<summary>Diff</summary>

```diff
<div class='iui-input-with-icon'>
<div
role='combobox'
tabindex='0'
class='iui-select-button iui-field'
+ data-iui-multi
>
- <span class='iui-content'>
- <div class='iui-select-tag-container'>…</div>
- </span>
Comment on lines +18 to +20
Copy link
Member

Choose a reason for hiding this comment

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

nit: the indentation can be changed to make it easier to read.

Suggested change
- <span class='iui-content'>
- <div class='iui-select-tag-container'>…</div>
- </span>
- <span class='iui-content'>
- <div class='iui-select-tag-container'>…</div>
- </span>

</div>
<svg class='iui-end-icon'>…</svg>
+ <span class='iui-content'>
+ <div class='iui-select-tag-container'>…</div>
+ </span>
</div>
```

</details>
Copy link
Member

@r100-stack r100-stack Nov 18, 2024

Choose a reason for hiding this comment

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

For the small size, I noticed that the ::before for the higher click target is not present. Also, the text seems larger. Are these some issues on my end?

EDIT: I think the selectors near this one need to be updated:

.iui-select-button[data-iui-size='small'] & {

Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
67 changes: 33 additions & 34 deletions apps/css-workshop/src/pages/select.astro
Original file line number Diff line number Diff line change
Expand Up @@ -507,56 +507,55 @@ import Icon_ from '../components/Icon.astro';

<section id='demo-multi'>
<div class='iui-input-with-icon'>
<div tabindex='0' class='iui-select-button iui-field' data-iui-size='small'>
<span class='iui-content'>
<div class='iui-select-tag-container'>
<multi-select-tag value='Option 1' dismissible></multi-select-tag>
<multi-select-tag value='Option 2' dismissible></multi-select-tag>
<multi-select-tag value='Option 3' dismissible></multi-select-tag>
</div>
</span>
<div tabindex='0' class='iui-select-button iui-field' data-iui-size='small' data-iui-multi>
</div>
<Icon_ svg='caret-down-small' class='iui-end-icon' />
<span class='iui-content'>
<div class='iui-select-tag-container'>
<multi-select-tag value='Option 1' dismissible></multi-select-tag>
<multi-select-tag value='Option 2' dismissible></multi-select-tag>
<multi-select-tag value='Option 3' dismissible></multi-select-tag>
</div>
</span>
</div>

<div class='iui-input-with-icon'>
<div tabindex='0' class='iui-select-button iui-field'>
<span class='iui-content'>
<div class='iui-select-tag-container'>
<multi-select-tag value='Option 1' dismissible></multi-select-tag>
<multi-select-tag value='Option 2' dismissible></multi-select-tag>
<multi-select-tag value='Option 3' dismissible></multi-select-tag>
</div>
</span>
</div>
<div tabindex='0' class='iui-select-button iui-field' data-iui-multi></div>
<Icon_ svg='caret-down-small' class='iui-end-icon' />
<span class='iui-content'>
<div class='iui-select-tag-container'>
<multi-select-tag value='Option 1' dismissible></multi-select-tag>
<multi-select-tag value='Option 2' dismissible></multi-select-tag>
<multi-select-tag value='Option 3' dismissible></multi-select-tag>
</div>
</span>
</div>

<div class='iui-input-with-icon'>
<div tabindex='0' class='iui-select-button iui-field' data-iui-size='large'>
<span class='iui-content'>
<div class='iui-select-tag-container'>
<multi-select-tag value='Option 1' dismissible></multi-select-tag>
<multi-select-tag value='Option 2' dismissible></multi-select-tag>
<multi-select-tag value='Option 3' dismissible></multi-select-tag>
</div>
</span>
<div tabindex='0' class='iui-select-button iui-field' data-iui-size='large' data-iui-multi>
</div>
<Icon_ svg='caret-down-small' class='iui-end-icon' />
<span class='iui-content'>
<div class='iui-select-tag-container'>
<multi-select-tag value='Option 1' dismissible></multi-select-tag>
<multi-select-tag value='Option 2' dismissible></multi-select-tag>
<multi-select-tag value='Option 3' dismissible></multi-select-tag>
</div>
</span>
</div>

<div class='iui-input-with-icon' style='width: 480px'>
<div tabindex='0' class='iui-select-button iui-field' data-iui-size='large'>
<span class='iui-content'>
<div class='iui-select-tag-container'>
<multi-select-tag value='Option 1' dismissible></multi-select-tag>
<multi-select-tag value='Option 2' dismissible></multi-select-tag>
<multi-select-tag value='Option 3' dismissible></multi-select-tag>
<multi-select-tag value='+5 more'></multi-select-tag>
</div>
</span>
<div tabindex='0' class='iui-select-button iui-field' data-iui-size='large' data-iui-multi>
</div>
<Icon_ svg='caret-down-small' class='iui-end-icon' />
<span class='iui-content'>
<div class='iui-select-tag-container'>
<multi-select-tag value='Option 1' dismissible></multi-select-tag>
<multi-select-tag value='Option 2' dismissible></multi-select-tag>
<multi-select-tag value='Option 3' dismissible></multi-select-tag>
<multi-select-tag value='+5 more'></multi-select-tag>
</div>
</span>
</div>
</section>
</Layout>
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
14 changes: 14 additions & 0 deletions packages/itwinui-css/src/select/select.scss
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,18 @@
color: inherit;
}

&:where([data-iui-multi]) ~ :where(.iui-content) {
grid-area: 1 / -1;
padding-inline-start: var(--iui-size-s);
padding-inline-end: var(--iui-component-height);
z-index: 1;
pointer-events: none; // to prevent blocking clicks on the select-button

> * {
pointer-events: auto; // this is mainly for custom renderer (as a fallback).
}
}

&.iui-placeholder,
&:has(option[value='']:checked) {
color: var(--iui-color-text-placeholder);
Expand Down Expand Up @@ -88,6 +100,7 @@
align-items: center;
gap: var(--iui-size-2xs);
overflow: hidden;
pointer-events: none; // to prevent blocking clicks on the select-button (nested container)

> * + * {
margin-inline-start: var(--iui-size-2xs);
Expand All @@ -105,6 +118,7 @@
block-size: 80%;
max-inline-size: 100%;
max-block-size: calc(var(--iui-size-s) * 3);
pointer-events: auto; // parent has pointer-events: none
}

@mixin iui-select-tag-label {
Expand Down
8 changes: 4 additions & 4 deletions packages/itwinui-react/src/core/Select/Select.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -516,13 +516,13 @@ it('should use custom render for selected item (multiple)', async () => {
<Select
value={['green', 'red']}
selectedItemRenderer={(options) => (
<>
<div className='selected-container'>
{options.map((option) => (
<span key={option.label} style={{ color: option?.value }}>
{option.label}
</span>
))}
</>
</div>
)}
options={[
{ value: 'yellow', label: 'Yellow' },
Expand All @@ -534,7 +534,7 @@ it('should use custom render for selected item (multiple)', async () => {
);

const selectedValues = container.querySelectorAll(
'.iui-select-button > span',
'.selected-container > span',
);
expect(selectedValues.length).toBe(2);
expect((selectedValues[0] as HTMLElement).style.color).toEqual('green');
Expand Down Expand Up @@ -612,7 +612,7 @@ it.each([true, false] as const)(

// deselect option A
if (multiple) {
expect(selectButton).toHaveTextContent('AB');
expect(selectButton).toHaveTextContent('A, B');
await userEvent.click((await findAllByRole(document.body, 'option'))[0]);
}

Expand Down
39 changes: 20 additions & 19 deletions packages/itwinui-react/src/core/Select/Select.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -456,23 +456,14 @@ const CustomSelect = React.forwardRef((props, forwardedRef) => {
},
triggerProps?.className,
)}
data-iui-multi={multiple}
>
{(!selectedItems || selectedItems.length === 0) && (
<Box as='span' className='iui-content'>
{placeholder}
</Box>
)}
{isMultipleEnabled(selectedItems, multiple) ? (
<MultipleSelectButton
selectedItems={selectedItems}
selectedItemsRenderer={
selectedItemRenderer as (
options: SelectOption<unknown>[],
) => JSX.Element
}
tagRenderer={tagRenderer}
/>
) : (
{!isMultipleEnabled(selectedItems, multiple) ? (
<SingleSelectButton
selectedItem={selectedItems}
selectedItemRenderer={
Expand All @@ -481,12 +472,21 @@ const CustomSelect = React.forwardRef((props, forwardedRef) => {
) => JSX.Element
}
/>
) : (
<AutoclearingHiddenLiveRegion text={liveRegionSelection} />
)}
</SelectButton>
<SelectEndIcon disabled={disabled} isOpen={isOpen} />

{multiple ? (
<AutoclearingHiddenLiveRegion text={liveRegionSelection} />
{isMultipleEnabled(selectedItems, multiple) ? (
<MultipleSelectButton
selectedItems={selectedItems}
selectedItemsRenderer={
selectedItemRenderer as (
options: SelectOption<unknown>[],
) => JSX.Element
}
tagRenderer={tagRenderer}
/>
) : null}
</InputWithIcon>

Expand Down Expand Up @@ -774,12 +774,13 @@ const MultipleSelectButton = <T,>({

return (
<>
{selectedItems &&
selectedItemsRenderer &&
selectedItemsRenderer(selectedItems)}
{selectedItems && !selectedItemsRenderer && (
{selectedItems && (
<Box as='span' className='iui-content'>
<SelectTagContainer tags={selectedItemsElements} />
{selectedItemsRenderer ? (
selectedItemsRenderer(selectedItems)
) : (
<SelectTagContainer tags={selectedItemsElements} />
)}
</Box>
)}
</>
Expand Down
Loading