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

fix(listbox): use filter tag #5952

Closed
wants to merge 17 commits into from
Closed

fix(listbox): use filter tag #5952

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
0dd3e5e
fix(listbox): use filter tag
janhassel Apr 28, 2020
8e8bd9c
fix(listbox): restore onKeyDown handler
janhassel Apr 28, 2020
dd1c100
fix(listbox): fix tag spacing on filterable multiselect
janhassel Apr 30, 2020
2327f5f
fix(listbox): remove explicit aria-label from tag
janhassel Apr 30, 2020
2718b37
Merge branch 'master' into listbox-tag
janhassel Apr 30, 2020
d859ed4
test(listbox): update snapshots
janhassel Apr 30, 2020
81d51e5
test(listbox): update snapshots
janhassel Apr 30, 2020
66270e5
test(listbox): update selectors
janhassel Apr 30, 2020
817907f
fix(listbox): apply correct styling to disabled tag
janhassel Apr 30, 2020
b7c779c
chore(listbox): fix prettier issue
janhassel Apr 30, 2020
481e914
fix(listbox): pass disabled state to tag
janhassel May 1, 2020
dcb9f62
Merge branch 'master' into listbox-tag
janhassel May 4, 2020
5dd96db
Merge branch 'master' into listbox-tag
janhassel May 18, 2020
e90ccfa
fix(multi-select): correct clear input button for filterable multiselect
janhassel May 31, 2020
5400128
Merge branch 'master' into listbox-tag
janhassel May 31, 2020
abc0b98
chore(multi-select): fix prettier issue
janhassel May 31, 2020
d62061a
Merge branch 'master' into listbox-tag
laurenmrice Jun 1, 2020
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
18 changes: 10 additions & 8 deletions packages/components/docs/sass.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7263,7 +7263,6 @@ $inverse-02: if(
- **Type**: `{undefined}`
- **Used by**:
- [carbon--theme [mixin]](#carbon--theme-mixin)
- [listbox [mixin]](#listbox-mixin)
- [inline-notifications [mixin]](#inline-notifications-mixin)
- [toast-notifications [mixin]](#toast-notifications-mixin)
- [tags [mixin]](#tags-mixin)
Expand Down Expand Up @@ -8189,6 +8188,7 @@ $disabled-03: if(
- [button [mixin]](#button-mixin)
- [button-base [mixin]](#button-base-mixin)
- [content-switcher [mixin]](#content-switcher-mixin)
- [listbox [mixin]](#listbox-mixin)
- [tabs [mixin]](#tabs-mixin)

### ✅highlight [variable]
Expand Down Expand Up @@ -18916,24 +18916,24 @@ List box styles
}

// Modifier for a selection to show that multiple selections have been made
.#{$prefix}--list-box__selection--multi {
@include type-style('label-01');
.#{$prefix}--tag.#{$prefix}--list-box__selection--multi {
position: static;
display: flex;
align-items: center;
justify-content: space-between;
padding: 0;
background-color: $inverse-02;
height: rem(24px);
width: auto;
color: $inverse-01;
line-height: 0;
padding: rem(8px);
padding-right: rem(2px); // Align with hover circle of X button
margin: 0;
margin-right: rem(10px);
border-radius: rem(12px);
}

.#{$prefix}--tag.#{$prefix}--list-box__selection--disabled {
@include tag-theme($disabled-02, $disabled-03);
}

.#{$prefix}--list-box__selection--multi > svg {
fill: $inverse-01;
margin-left: rem(4px);
Expand Down Expand Up @@ -19228,6 +19228,7 @@ List box styles

- **Group**: [list-box](#list-box)
- **Requires**:
- [tag-theme [mixin]](#tag-theme-mixin)
- [carbon--mini-units [function]](#carbon--mini-units-function)
- [prefix [variable]](#prefix-variable)
- [list-box-width [variable]](#list-box-width-variable)
Expand All @@ -19247,7 +19248,7 @@ List box styles
- [carbon--spacing-05 [variable]](#carbon--spacing-05-variable)
- [icon-01 [variable]](#icon-01-variable)
- [icon-02 [variable]](#icon-02-variable)
- [inverse-02 [variable]](#inverse-02-variable)
- [disabled-03 [variable]](#disabled-03-variable)
- [inverse-01 [variable]](#inverse-01-variable)
- [hover-secondary [variable]](#hover-secondary-variable)
- [ui-01 [variable]](#ui-01-variable)
Expand Down Expand Up @@ -23329,6 +23330,7 @@ Tabs styles
- **Requires**:
- [prefix [variable]](#prefix-variable)
- **Used by**:
- [listbox [mixin]](#listbox-mixin)
- [tags [mixin]](#tags-mixin)

### ❌tags [mixin]
Expand Down
23 changes: 7 additions & 16 deletions packages/components/src/components/list-box/_list-box.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
@import '../../globals/scss/helper-mixins';
@import '../../globals/scss/layout';
@import '../../globals/scss/vendor/@carbon/elements/scss/import-once/import-once';
@import '../tag/tag';
@import '../../globals/scss/css--reset';

/// @type Number
Expand Down Expand Up @@ -382,24 +383,24 @@ $list-box-menu-width: rem(300px);
}

// Modifier for a selection to show that multiple selections have been made
.#{$prefix}--list-box__selection--multi {
@include type-style('label-01');
.#{$prefix}--tag.#{$prefix}--list-box__selection--multi {
position: static;
display: flex;
align-items: center;
justify-content: space-between;
padding: 0;
background-color: $inverse-02;
height: rem(24px);
width: auto;
color: $inverse-01;
line-height: 0;
padding: rem(8px);
padding-right: rem(2px); // Align with hover circle of X button
margin: 0;
margin-right: rem(10px);
border-radius: rem(12px);
}

.#{$prefix}--tag.#{$prefix}--list-box__selection--disabled {
@include tag-theme($disabled-02, $disabled-03);
}

.#{$prefix}--list-box__selection--multi > svg {
fill: $inverse-01;
margin-left: rem(4px);
Expand All @@ -413,16 +414,6 @@ $list-box-menu-width: rem(300px);
}
}

.#{$prefix}--list-box--disabled
.#{$prefix}--list-box__selection--multi
> svg {
fill: $disabled-02;

&:hover {
background-color: unset;
}
}

.#{$prefix}--list-box__selection--multi:focus,
.#{$prefix}--list-box__selection--multi:hover {
outline: none;
Expand Down
28 changes: 28 additions & 0 deletions packages/components/src/components/multi-select/_multi-select.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,34 @@
}
}

.#{$prefix}--multi-select__clear-input {
position: absolute;
right: rem(33px); // to preserve .5rem space between icons according to spec
// top/transform used to center the combobox clear selection icon in IE11
top: 50%;
transform: translateY(-50%);
display: flex;
justify-content: center;
align-items: center;
height: rem(30px);
width: rem(30px);
cursor: pointer;
user-select: none;

&:focus {
@include focus-outline('outline');
}
}

.#{$prefix}--multi-select__clear-input--disabled {
cursor: not-allowed;
pointer-events: none;
}

.#{$prefix}--multi-select__clear-input--disabled > svg {
fill: $disabled-02;
}

.#{$prefix}--multi-select--selected .#{$prefix}--text-input {
// this value will need to change based on the number of digits in
// the number of items selected
Expand Down
4 changes: 0 additions & 4 deletions packages/components/src/components/tag/_tag.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,10 +190,6 @@
background-color: transparent;
}

.#{$prefix}--tag--filter.#{$prefix}--tag--disabled svg {
fill: $disabled-02;
}

// Skeleton state
.#{$prefix}--tag.#{$prefix}--skeleton {
@include skeleton;
Expand Down
18 changes: 9 additions & 9 deletions packages/react/src/components/ListBox/ListBoxSelection.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@
import cx from 'classnames';
import React from 'react';
import PropTypes from 'prop-types';
import { Close16 } from '@carbon/icons-react';
import { settings } from 'carbon-components';
import { match, keys } from '../../internal/keyboard';
import Tag from '../Tag';

const { prefix } = settings;

Expand All @@ -27,8 +27,8 @@ const ListBoxSelection = ({
onClearSelection,
}) => {
const className = cx(`${prefix}--list-box__selection`, {
[`${prefix}--tag--filter`]: selectionCount,
[`${prefix}--list-box__selection--multi`]: selectionCount,
[`${prefix}--list-box__selection--disabled`]: disabled,
});
const handleOnClick = event => {
event.stopPropagation();
Expand Down Expand Up @@ -56,17 +56,17 @@ const ListBoxSelection = ({
};
const description = selectionCount ? t('clear.all') : t('clear.selection');
return (
<div
role="button"
<Tag
Copy link
Member

Choose a reason for hiding this comment

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

I think we can add the disabled prop here so we can get the correct styles when the multiselect is disabled

Copy link
Member Author

Choose a reason for hiding this comment

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

I was thinking that too and compared it to the current implementation where the tag styling is not changed on disable.

Personally I would also prefer to have the tag disabled since otherwise it really stands out. Do you if there was a reason behind the current implementation, like keeping it well readable for accessibility maybe?

Copy link
Member

Choose a reason for hiding this comment

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

it looks like there is a slight color change on the disabled tag svg in the current multiselect, but I'm not sure if it should look like the standalone disabled tag styles cc @laurenmrice

Copy link
Member Author

Choose a reason for hiding this comment

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

Just as a reference: this is how it would look when the tag enters disabled state as well:
image

Copy link
Member

@laurenmrice laurenmrice Apr 30, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Let me make a spec for this 👍🏻, will get back to you shortly

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 what we can do for a selected-disabled state.

tags

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the quick specs, @laurenmrice 👍
I updated the tag color for the disabled state accordingly, you can check it out in the deploy preview: https://5eaae7cd3ab6150007c0340b--carbon-components-react.netlify.app/?path=/story/multiselect--default

className={className}
tabIndex={disabled ? -1 : 0}
type="high-contrast"
filter
onClose={handleOnClick}
onClick={handleOnClick}
onKeyDown={handleOnKeyDown}
aria-label="Clear Selection"
title={description}>
title={description}
disabled={disabled}>
{selectionCount}
<Close16 />
</div>
</Tag>
);
};

Expand Down
146 changes: 92 additions & 54 deletions packages/react/src/components/ListBox/__tests__/__snapshots__/ListBoxField-test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,39 +16,58 @@ exports[`ListBoxField should render 1`] = `
clearSelection={[MockFunction]}
translateWithId={[Function]}
>
<div
aria-label="Clear Selection"
<Tag
className="bx--list-box__selection"
filter={true}
onClick={[Function]}
onClose={[Function]}
onKeyDown={[Function]}
role="button"
tabIndex={0}
title="Clear selected item"
type="high-contrast"
>
<ForwardRef(Close16)>
<Icon
height={16}
preserveAspectRatio="xMidYMid meet"
viewBox="0 0 32 32"
width={16}
xmlns="http://www.w3.org/2000/svg"
<div
aria-label="Clear selected item undefined"
className="bx--tag bx--list-box__selection bx--tag--filter bx--tag--high-contrast"
id="tag-1"
onClick={[Function]}
onKeyDown={[Function]}
>
<span
className="bx--tag__label"
>
<svg
aria-hidden={true}
focusable="false"
height={16}
preserveAspectRatio="xMidYMid meet"
viewBox="0 0 32 32"
width={16}
xmlns="http://www.w3.org/2000/svg"
>
<path
d="M24 9.4L22.6 8 16 14.6 9.4 8 8 9.4 14.6 16 8 22.6 9.4 24 16 17.4 22.6 24 24 22.6 17.4 16 24 9.4z"
/>
</svg>
</Icon>
</ForwardRef(Close16)>
</div>
High-Contrast
</span>
<button
aria-labelledby="tag-1"
className="bx--tag__close-icon"
onClick={[Function]}
>
<ForwardRef(Close16)>
<Icon
height={16}
preserveAspectRatio="xMidYMid meet"
viewBox="0 0 32 32"
width={16}
xmlns="http://www.w3.org/2000/svg"
>
<svg
aria-hidden={true}
focusable="false"
height={16}
preserveAspectRatio="xMidYMid meet"
viewBox="0 0 32 32"
width={16}
xmlns="http://www.w3.org/2000/svg"
>
<path
d="M24 9.4L22.6 8 16 14.6 9.4 8 8 9.4 14.6 16 8 22.6 9.4 24 16 17.4 22.6 24 24 22.6 17.4 16 24 9.4z"
/>
</svg>
</Icon>
</ForwardRef(Close16)>
</button>
</div>
</Tag>
</ListBoxSelection>
</div>
</ListBoxField>
Expand All @@ -72,39 +91,58 @@ exports[`ListBoxField should set \`aria-owns\` based when expanded 1`] = `
clearSelection={[MockFunction]}
translateWithId={[Function]}
>
<div
aria-label="Clear Selection"
<Tag
className="bx--list-box__selection"
filter={true}
onClick={[Function]}
onClose={[Function]}
onKeyDown={[Function]}
role="button"
tabIndex={0}
title="Clear selected item"
type="high-contrast"
>
<ForwardRef(Close16)>
<Icon
height={16}
preserveAspectRatio="xMidYMid meet"
viewBox="0 0 32 32"
width={16}
xmlns="http://www.w3.org/2000/svg"
<div
aria-label="Clear selected item undefined"
className="bx--tag bx--list-box__selection bx--tag--filter bx--tag--high-contrast"
id="tag-3"
onClick={[Function]}
onKeyDown={[Function]}
>
<span
className="bx--tag__label"
>
High-Contrast
</span>
<button
aria-labelledby="tag-3"
className="bx--tag__close-icon"
onClick={[Function]}
>
<svg
aria-hidden={true}
focusable="false"
height={16}
preserveAspectRatio="xMidYMid meet"
viewBox="0 0 32 32"
width={16}
xmlns="http://www.w3.org/2000/svg"
>
<path
d="M24 9.4L22.6 8 16 14.6 9.4 8 8 9.4 14.6 16 8 22.6 9.4 24 16 17.4 22.6 24 24 22.6 17.4 16 24 9.4z"
/>
</svg>
</Icon>
</ForwardRef(Close16)>
</div>
<ForwardRef(Close16)>
<Icon
height={16}
preserveAspectRatio="xMidYMid meet"
viewBox="0 0 32 32"
width={16}
xmlns="http://www.w3.org/2000/svg"
>
<svg
aria-hidden={true}
focusable="false"
height={16}
preserveAspectRatio="xMidYMid meet"
viewBox="0 0 32 32"
width={16}
xmlns="http://www.w3.org/2000/svg"
>
<path
d="M24 9.4L22.6 8 16 14.6 9.4 8 8 9.4 14.6 16 8 22.6 9.4 24 16 17.4 22.6 24 24 22.6 17.4 16 24 9.4z"
/>
</svg>
</Icon>
</ForwardRef(Close16)>
</button>
</div>
</Tag>
</ListBoxSelection>
</div>
</ListBoxField>
Expand Down
Loading