feat: adding count label on multiselect component #865

alok-traceable · 2021-05-20T06:25:40Z

Description

Current behaviour of Multiselect component label is and more.
This PR contains changes which will Instead of showing "and " will show "+" tag with a grey background and border radius.

Testing

Unit tests to verify the existence of the tag and its value.

Checklist:

My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
Any dependent changes have been merged and published in downstream modules

codecov · 2021-05-20T06:28:55Z

Codecov Report

Merging #865 (c55b87a) into main (7cc5e77) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #865      +/-   ##
==========================================
- Coverage   85.36%   85.36%   -0.01%     
==========================================
  Files         799      799              
  Lines       16435    16433       -2     
  Branches     2064     2064              
==========================================
- Hits        14030    14028       -2     
  Misses       2373     2373              
  Partials       32       32

Impacted Files	Coverage Δ
...ponents/src/multi-select/multi-select.component.ts	`98.43% <100.00%> (-0.05%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7cc5e77...c55b87a. Read the comment docs.

itssharmasandeep · 2021-05-20T07:22:48Z

projects/components/src/multi-select/multi-select.component.scss

+        padding: 0 6px;
+        background-color: $gray-2;
+        border-radius: 4px;
+        margin-left: 5px;


I guess, we use margin/padding as multiple of 4. You may check that.
Otherwise looks good to me!!

…font size

projects/components/src/multi-select/multi-select.component.scss

projects/components/src/multi-select/multi-select.component.ts

anandtiwary · 2021-05-24T15:51:45Z

projects/components/src/multi-select/multi-select.component.test.ts

-    expect(spectator.query(LabelComponent)?.label).toEqual('first and 5 more');
+    expect(spectator.query(LabelComponent)?.label).toEqual('first');
+
+    expect(spectator.component.selectedItemsCount).toBe(6);


We should test this from Dom. Example:

expect(spectator.query('.trigger-more-items')).toHaveText('6')

aaron-steinfeld · 2021-05-25T13:52:33Z

projects/components/src/multi-select/multi-select.component.ts

@@ -236,10 +240,13 @@ export class MultiSelectComponent<V> implements AfterContentInit, OnChanges {
    );
    if (selectedItems === undefined || selectedItems.length === 0) {
      this.triggerLabel = this.placeholder;
+      this.selectedItemsCount = 0;


nit: can move this assignment out of the condition then collapse the conditional

this.selectedItemsCount = selectedItems?.length ?? 0; this.triggerLabel = this.selectedItemsCount === 0 ? this.placeholder : selectedItems[0].label;

aaron-steinfeld · 2021-05-25T13:53:21Z

projects/components/src/multi-select/multi-select.component.scss

+        margin-left: 8px;
+        max-width: 38px;
+        font-size: 12px;
+        font-weight: 600;


can we pull this font from our shared font styles? We generally want all fonts to be defined in font.scss so we're not defining them adhoc in every style sheet.

github-actions · 2021-05-25T17:17:46Z

Unit Test Results

    4 files ±0 254 suites ±0 13m 48s ⏱️ -19s
916 tests ±0 916 ✔️ ±0 0 💤 ±0 0 ❌ ±0
922 runs ±0 922 ✔️ ±0 0 💤 ±0 0 ❌ ±0

Results for commit c452358. ± Comparison against base commit 7cc5e77.

Alok Singh added 2 commits May 20, 2021 12:57

feat: adding count label on multiselect component

0dadf59

feat: adding count label on multiselect component

674363d

alok-traceable requested review from aaron-steinfeld, anandtiwary and arjunlalb May 20, 2021 06:25

alok-traceable requested a review from a team as a code owner May 20, 2021 06:25

This comment has been minimized.

Sign in to view

feat: css changes for width calculation

bacbd0f

This comment has been minimized.

Sign in to view

itssharmasandeep reviewed May 20, 2021

View reviewed changes

Alok Singh added 2 commits May 20, 2021 23:43

feat: pr suggested chages for spacing in multiple of 4 and reduceing …

d9260f3

…font size

feat: increasing font wight to 600 for consistancy

c9d675a

This comment has been minimized.

Sign in to view

anandtiwary reviewed May 20, 2021

View reviewed changes

projects/components/src/multi-select/multi-select.component.scss Show resolved Hide resolved

anandtiwary reviewed May 20, 2021

View reviewed changes

projects/components/src/multi-select/multi-select.component.ts Show resolved Hide resolved

alok-traceable requested review from itssharmasandeep and anandtiwary May 21, 2021 03:59

anandtiwary reviewed May 24, 2021

View reviewed changes

fix: pr suggested changes

58a3461

This comment has been minimized.

Sign in to view

aaron-steinfeld reviewed May 25, 2021

View reviewed changes

Alok Singh added 2 commits May 25, 2021 23:01

Merge branch 'main' into multi_select_count_label

50a7f23

fix: pr suggested changes

e98ba46

This comment has been minimized.

Sign in to view

fix: pr suggested changes

4a78354

This comment has been minimized.

Sign in to view

fix: pr suggested changes

c55b87a

aaron-steinfeld approved these changes May 25, 2021

View reviewed changes

This comment has been minimized.

Sign in to view

alok-traceable merged commit c452358 into main May 25, 2021

alok-traceable deleted the multi_select_count_label branch May 25, 2021 17:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: adding count label on multiselect component #865

feat: adding count label on multiselect component #865

Uh oh!

alok-traceable commented May 20, 2021

Uh oh!

codecov bot commented May 20, 2021 •

edited

Loading

Uh oh!

This comment has been minimized.

This comment has been minimized.

itssharmasandeep May 20, 2021

Uh oh!

alok-traceable May 20, 2021

Uh oh!

This comment has been minimized.

This comment has been minimized.

Uh oh!

Uh oh!

anandtiwary May 24, 2021

Uh oh!

alok-traceable May 25, 2021

Uh oh!

This comment has been minimized.

aaron-steinfeld May 25, 2021

Uh oh!

alok-traceable May 25, 2021

Uh oh!

aaron-steinfeld May 25, 2021

Uh oh!

alok-traceable May 25, 2021

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions bot commented May 25, 2021

Uh oh!

Uh oh!

feat: adding count label on multiselect component #865

feat: adding count label on multiselect component #865

Uh oh!

Conversation

alok-traceable commented May 20, 2021

Description

Testing

Checklist:

Uh oh!

codecov bot commented May 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

This comment has been minimized.

This comment has been minimized.

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions bot commented May 25, 2021

Unit Test Results

Uh oh!

Uh oh!

codecov bot commented May 20, 2021 •

edited

Loading