[CL-221] Add chip-select component #9021

vleague2 · 2024-05-02T16:51:02Z

Type of change

- [ ] Bug fix
- [x] New feature development
- [ ] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ ] Other

Objective

This PR adds a new CL component: bit-chip-select

Non-goals: full compliance with Figma design spec--further design and a11y changes will occur in a separate PR off of #8696

Code changes

libs/components/src/chip-select: New component, docs, stories
libs/components/src/menu: Changes to facilitate truncation, start + end slots, disabled state
libs/components/src/option: Changes to type signature to more accurately convey what is required

Screenshots

See Storybook for more

Screen.Recording.2024-05-21.at.10.04.12.PM.mov

Before you submit

Please add unit tests where it makes sense to do so (encouraged but not required)
If this change requires a documentation update - notify the documentation team
If this change has particular deployment requirements - notify the DevOps team
Ensure that all UI additions follow WCAG AA requirements

codecov · 2024-05-02T16:57:45Z

Codecov Report

Attention: Patch coverage is 2.53165% with 77 lines in your changes are missing coverage. Please review.

Project coverage is 28.15%. Comparing base (b0cc480) to head (4bd0d15).
Report is 30 commits behind head on main.

Files	Patch %	Lines
...omponents/src/chip-select/chip-select.component.ts	0.00%	53 Missing ⚠️
.../components/src/chip-select/chip-select.stories.ts	0.00%	21 Missing ⚠️
libs/components/src/chip-select/index.ts	0.00%	1 Missing ⚠️
libs/components/src/index.ts	0.00%	1 Missing ⚠️
libs/components/src/select/option.component.ts	0.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##             main    #9021    +/-   ##
========================================
  Coverage   28.15%   28.15%            
========================================
  Files        2369     2375     +6     
  Lines       70028    70220   +192     
  Branches    13160    13172    +12     
========================================
+ Hits        19718    19773    +55     
- Misses      48751    48882   +131     
- Partials     1559     1565     +6

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

danielleflinn

For an initial commit this looks good.

In Figma I have the chip text-size as body2. We might want to update that now since it doesn't rely on any new styles. But I'll leave that up to you @willmartian if you'd prefer to tackle it in the follow up one.

We should address the following in a followup PR:

chip focus state
chip styles for hover, pressed, and focused states
keyboard navigation and SR announcements
style updates that will happen as part of updates to the bit-menu

willmartian · 2024-05-22T20:10:20Z

Thanks @danielleflinn! I went ahead and added a focus ring and updated the font size to use body2. Will finalize the other items in a separate ticket.

danielleflinn · 2024-05-22T21:47:42Z

@willmartian I just noticed in the menu the list focus is only on the top of the item, but in the bit-menu story we have it on both the top and bottom. Is this expected? Will it be addressed in the follow up PR?

vleague2

Ty for such beautiful work ✨ Here are a couple of questions!

libs/components/src/chip-select/chip-select.component.html

libs/components/src/chip-select/chip-select.component.ts

libs/components/src/chip-select/chip-select.component.html

vleague2 · 2024-05-23T13:43:07Z

libs/components/src/chip-select/chip-select.component.html

+ type="button"
+ [attr.aria-label]="'removeItem' | i18n: label"
+ [disabled]="disabled"
+ class="tw-bg-transparent hover:tw-bg-transparent tw-outline-none tw-rounded-full tw-p-1 tw-my-1 tw-mr-1 tw-text-[inherit] tw-border-solid tw-border tw-border-text-muted hover:tw-border-text-contrast hover:disabled:tw-border-transparent tw-aspect-square tw-flex tw-items-center tw-justify-center tw-h-fit focus-visible:tw-ring-2 tw-ring-text-contrast focus-visible:hover:tw-border-transparent"


The focus ring is kind of oblong?

Huh, I am not seeing that in FF, but I am on Chrome. Will solve that in the follow up PR for CL-291.

That screenshot is from Chrome 😭

libs/components/src/select/option.component.ts

willmartian · 2024-05-23T17:33:15Z

I just noticed in the menu the list focus is only on the top of the item, but in the bit-menu story we have it on both the top and bottom. Is this expected? Will it be addressed in the follow up PR?

@danielleflinn I believe this is a style bug with the menu component itself. I would like to care of that when we update menu styles in CL-237 if that is cool? (Also, the focus rings in the menu component are also not up to spec on main.)

libs/components/src/chip-select/chip-select.component.html

vleague2

✅ This is me symbolically approving the PR because I can't actually approve it since I own it

willmartian

Wow, amazing work

willmartian

Oh actually, it looks like the menu changes here introduces a regression in the product switcher. Gonna fix this real quick.

edit: fixed in 4bd0d15

vleague2

✅ Width changes look good

testing icon sizes

11a60c8

github-actions bot added the needs-qa Marks a PR as requiring QA approval label May 2, 2024

This comment was marked as off-topic.

Sign in to view

rename, add menu, update styling

f7514ec

vleague2 changed the title ~~testing icon sizes~~ [CL-221] Add chip-select component May 3, 2024

willmartian added 13 commits May 9, 2024 14:02

rename class

e66e0b7

renamed template file

d2b59a2

fix templateUrl

864d1e7

wip

41a59da

add truncation and close icon

cac7624

wip; implement form value accessor

e6d3f2c

add slots and truncation to bitMenuItem

86cae84

add disabled button styles to bitMenuItem

8aced14

wip

b5673a8

update docs

e71e260

add play function to stories

c6c1f3a

add i18n

a64a634

resolve merge conflicts

bdd7d06

willmartian marked this pull request as ready for review May 22, 2024 02:17

willmartian requested a review from a team as a code owner May 22, 2024 02:17

willmartian requested a review from shane-melton May 22, 2024 02:18

willmartian removed the needs-qa Marks a PR as requiring QA approval label May 22, 2024

willmartian self-assigned this May 22, 2024

willmartian added 5 commits May 22, 2024 12:42

update comments

00c17f0

add disabled styles; show selected value option upon opening menu

d6a231c

update i18n; use getters for placeholder content

bf63dbc

fix label bug in prev commit

4e03672

fix: update rendered options after value change

32e83b0

danielleflinn self-requested a review May 22, 2024 19:06

danielleflinn previously approved these changes May 22, 2024

View reviewed changes

willmartian added 2 commits May 22, 2024 15:55

style fixes

948ba12

add focus ring

143dc84

willmartian dismissed danielleflinn’s stale review via 143dc84 May 22, 2024 20:08

vleague2 commented May 23, 2024

View reviewed changes

willmartian added 2 commits May 23, 2024 12:27

update font-size in menu

2977cd5

init root tree in options setter

14edafb

use parent map instead of in-tree reference

05beb78

vleague2 commented May 23, 2024

View reviewed changes

libs/components/src/chip-select/chip-select.component.html Outdated Show resolved Hide resolved

vleague2 commented May 23, 2024

View reviewed changes

willmartian approved these changes May 23, 2024

View reviewed changes

willmartian requested changes May 23, 2024

View reviewed changes

remove max width from menu; add to chip-select menu only

4bd0d15

vleague2 commented May 23, 2024

View reviewed changes

willmartian approved these changes May 23, 2024

View reviewed changes

willmartian merged commit 08a6f91 into main May 23, 2024
62 checks passed

willmartian deleted the ps/cl-221/chip-select branch May 23, 2024 20:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CL-221] Add chip-select component #9021

[CL-221] Add chip-select component #9021

vleague2 commented May 2, 2024 •

edited by willmartian

codecov bot commented May 2, 2024 •

edited

This comment was marked as off-topic.

danielleflinn left a comment

willmartian commented May 22, 2024

danielleflinn commented May 22, 2024

vleague2 left a comment

vleague2 May 23, 2024

willmartian May 23, 2024

vleague2 May 23, 2024

willmartian commented May 23, 2024 •

edited

vleague2 left a comment

willmartian left a comment

willmartian left a comment •

edited

vleague2 left a comment

[CL-221] Add chip-select component #9021

[CL-221] Add chip-select component #9021

Conversation

vleague2 commented May 2, 2024 • edited by willmartian

Type of change

Objective

Code changes

Screenshots

Before you submit

codecov bot commented May 2, 2024 • edited

Codecov Report

This comment was marked as off-topic.

danielleflinn left a comment

Choose a reason for hiding this comment

willmartian commented May 22, 2024

danielleflinn commented May 22, 2024

vleague2 left a comment

Choose a reason for hiding this comment

vleague2 May 23, 2024

Choose a reason for hiding this comment

willmartian May 23, 2024

Choose a reason for hiding this comment

vleague2 May 23, 2024

Choose a reason for hiding this comment

willmartian commented May 23, 2024 • edited

vleague2 left a comment

Choose a reason for hiding this comment

willmartian left a comment

Choose a reason for hiding this comment

willmartian left a comment • edited

Choose a reason for hiding this comment

vleague2 left a comment

Choose a reason for hiding this comment

vleague2 commented May 2, 2024 •

edited by willmartian

codecov bot commented May 2, 2024 •

edited

willmartian commented May 23, 2024 •

edited

willmartian left a comment •

edited