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

feat(component): convert Dropdown/Select to FC and add MultiSelect #303

Merged
merged 7 commits into from
Feb 18, 2020

Conversation

jorgemoya
Copy link
Contributor

@jorgemoya jorgemoya commented Dec 23, 2019

** BREAKING CHANGES**

Changes

  1. Refactor Dropdown and Select to use Downshift. The goal is to leverage this library's accessibility logic. Prop breaking changes.
  2. Split MultiSelect to have its own component.
  3. Updated the way we handle Input and Chips for a more streamlined logic.

@jorgemoya jorgemoya requested a review from a team December 23, 2019 17:03
Copy link
Contributor

@chanceaclark chanceaclark left a comment

Choose a reason for hiding this comment

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

So about my concerns with module size: I did a analysis by bundling the es module with webpack, these are the stats I received:

368 KiB - [Downshift]
353 KiB - [w/o]

So that's if someone bundled the entire BigDesign library with their app. I'm not concerned with those numbers. Maybe we should do some more testing around that though?

packages/big-design/src/components/Dropdown/Dropdown.tsx Outdated Show resolved Hide resolved
packages/big-design/src/components/Dropdown/Dropdown.tsx Outdated Show resolved Hide resolved
packages/big-design/src/components/Select/Select.tsx Outdated Show resolved Hide resolved
packages/big-design/src/components/Dropdown/Dropdown.tsx Outdated Show resolved Hide resolved
packages/big-design/src/components/Dropdown/Dropdown.tsx Outdated Show resolved Hide resolved
packages/big-design/src/components/Select/styled.tsx Outdated Show resolved Hide resolved
packages/big-design/src/components/Tooltip/Tooltip.tsx Outdated Show resolved Hide resolved
packages/docs/PropTables/SelectPropTable.tsx Outdated Show resolved Hide resolved
packages/big-design/src/components/Dropdown/Dropdown.tsx Outdated Show resolved Hide resolved
packages/big-design/src/components/Dropdown/Dropdown.tsx Outdated Show resolved Hide resolved
packages/big-design/src/components/Dropdown/Dropdown.tsx Outdated Show resolved Hide resolved
packages/big-design/src/components/Dropdown/Dropdown.tsx Outdated Show resolved Hide resolved
packages/big-design/src/components/Input/Input.tsx Outdated Show resolved Hide resolved
packages/big-design/src/components/Select/Select.tsx Outdated Show resolved Hide resolved
packages/big-design/src/components/Select/Select.tsx Outdated Show resolved Hide resolved
openMenu();
},
onKeyDown: (event: any) => {
if (multi && event.key === 'Backspace' && !inputValue) {
Copy link
Member

Choose a reason for hiding this comment

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

Should we be checking for inputValue.length? Could there be a corner case with 0?

Copy link
Contributor Author

@jorgemoya jorgemoya Jan 2, 2020

Choose a reason for hiding this comment

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

Hm, I fail to see why it needs to change, tbh. inputValue is a string, and will only return falsey if it's "" right (which is what I want)?

packages/docs/PropTables/SelectPropTable.tsx Outdated Show resolved Hide resolved
@jorgemoya jorgemoya force-pushed the dropdown-refactor-v3 branch 4 times, most recently from a0cb9fe to 460248d Compare January 14, 2020 22:05
@jorgemoya jorgemoya changed the title refactor(component): refactor Dropdown/Select to use downshift-js feat(component): convert Dropdown/Select to FC and add MultiSelect Jan 15, 2020
@jorgemoya jorgemoya force-pushed the dropdown-refactor-v3 branch 4 times, most recently from 50cda46 to 16cadf5 Compare January 16, 2020 18:30
packages/big-design/src/components/Dropdown/Dropdown.tsx Outdated Show resolved Hide resolved
packages/big-design/src/components/List/List.tsx Outdated Show resolved Hide resolved
packages/big-design/src/components/Select/Select.tsx Outdated Show resolved Hide resolved
packages/big-design/src/components/Select/spec.tsx Outdated Show resolved Hide resolved
packages/docs/PropTables/SelectPropTable.tsx Outdated Show resolved Hide resolved
packages/docs/PropTables/SelectPropTable.tsx Outdated Show resolved Hide resolved
packages/docs/pages/MultiSelect/MultiSelectPage.tsx Outdated Show resolved Hide resolved
@jorgemoya jorgemoya force-pushed the dropdown-refactor-v3 branch 7 times, most recently from e5d2b3b to 526430d Compare January 22, 2020 21:47
@jorgemoya jorgemoya force-pushed the dropdown-refactor-v3 branch from 08e268a to ac1b891 Compare February 5, 2020 19:02
@jorgemoya jorgemoya force-pushed the dropdown-refactor-v3 branch 2 times, most recently from b46f91f to 6280bd9 Compare February 14, 2020 16:40
@jorgemoya jorgemoya force-pushed the dropdown-refactor-v3 branch from f6a8504 to 0f6e6e0 Compare February 18, 2020 17:23
Copy link
Member

@deini deini left a comment

Choose a reason for hiding this comment

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

Let's get this merged and iterate on it

@jorgemoya jorgemoya merged commit 0ab0e50 into bigcommerce:master Feb 18, 2020
@jorgemoya jorgemoya deleted the dropdown-refactor-v3 branch February 18, 2020 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants