Skip to content
This repository was archived by the owner on Mar 4, 2020. It is now read-only.

feat(dropdown): multiple selection#845

Merged
bmdalex merged 9 commits intomasterfrom
feat/dropdown-multiple-selection
Feb 6, 2019
Merged

feat(dropdown): multiple selection#845
bmdalex merged 9 commits intomasterfrom
feat/dropdown-multiple-selection

Conversation

@bmdalex
Copy link
Collaborator

@bmdalex bmdalex commented Feb 5, 2019

feat(dropdown): multiple selection

Description:

This PR implements the multiple selection with trigger button flavor for Dropdown component (easy to leverage existing functionality)

API

<Dropdown
  multiple
  items={inputItems}
  placeholder="Start typing a name"
  noResultsMessage="We couldn't find any matches."
/>

renders:
screen recording 2019-02-04 at 21 44 10

@bmdalex bmdalex force-pushed the feat/dropdown-multiple-selection branch from a52bbea to 472c222 Compare February 6, 2019 11:00
@codecov
Copy link

codecov bot commented Feb 6, 2019

Codecov Report

Merging #845 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #845   +/-   ##
=======================================
  Coverage   93.54%   93.54%           
=======================================
  Files          21       21           
  Lines         728      728           
  Branches       69       73    +4     
=======================================
  Hits          681      681           
  Misses         47       47

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 6a049c1...472c222. Read the comment docs.

@codecov
Copy link

codecov bot commented Feb 6, 2019

Codecov Report

Merging #845 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #845   +/-   ##
=======================================
  Coverage   93.54%   93.54%           
=======================================
  Files          21       21           
  Lines         728      728           
  Branches       73       73           
=======================================
  Hits          681      681           
  Misses         47       47

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 d89668f...d38f328. Read the comment docs.

@bmdalex bmdalex force-pushed the feat/dropdown-multiple-selection branch 2 times, most recently from b53d0d9 to 4701332 Compare February 6, 2019 13:59
Copy link
Collaborator

Choose a reason for hiding this comment

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

why do we need that parameter if we can get it from state as we did with the props? this method might as well be getTriggerButtonMessage or getTriggerButtonText. not important but was just curious why not like that. oh, and capital I in If there is no value...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this method is also used here, but not merged yet:
https://github.com/stardust-ui/react/pull/839/files#r254290634

with a different parameter, so the proposed name would not apply anymore, open to other naming suggestions

Will add capital "I", thanks

@bmdalex bmdalex force-pushed the feat/dropdown-multiple-selection branch from 4701332 to 581688e Compare February 6, 2019 14:49
@bmdalex bmdalex force-pushed the feat/dropdown-multiple-selection branch from 581688e to 604ccd5 Compare February 6, 2019 15:46
@bmdalex bmdalex merged commit 9559996 into master Feb 6, 2019
@bmdalex bmdalex deleted the feat/dropdown-multiple-selection branch February 7, 2019 14:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants