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

feat(dropdown): autocontrolled mode for open state #900

Merged
merged 4 commits into from
Feb 20, 2019

Conversation

bmdalex
Copy link
Collaborator

@bmdalex bmdalex commented Feb 14, 2019

feat(dropdown): autocontrolled mode for open state

Description:

  • introduced open, defaultOpen props
  • introduced onOpenChange event handler

API:

<Dropdown
  open={open}
  onOpenChange={handleOpenChange}
  items={inputItems}
/>

where open value is part of the state of HOC and handleOpenChange is the entry for internal changes of open.

Screenshot:

screen recording 2019-02-14 at 15 06 47

Blockers:

Currently blocked by possible Downshift issue related to internal state handling when isOpen prop is used in controlled mode.

The Dropdown still works but there is a regression when opening the list of items:

The first/last item is not highlighted by default anymore after introducing open prop.

Here is the expected screener regression

@silviuavram has more details

@bmdalex
Copy link
Collaborator Author

bmdalex commented Feb 14, 2019

Currently blocked by possible Downshift issue related to internal state handling when isOpen prop is used in controlled mode.

The Dropdown still works but there is a regression when opening the list of items:

The first/last item is not highlighted by default anymore after introducing open prop.

Here is the expected screener regression

@silviuavram has more details

@codecov
Copy link

codecov bot commented Feb 14, 2019

Codecov Report

Merging #900 into master will decrease coverage by 0.03%.
The diff coverage is 23.07%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #900      +/-   ##
==========================================
- Coverage   80.42%   80.38%   -0.04%     
==========================================
  Files         659      656       -3     
  Lines        8412     8392      -20     
  Branches     1488     1424      -64     
==========================================
- Hits         6765     6746      -19     
+ Misses       1632     1631       -1     
  Partials       15       15
Impacted Files Coverage Δ
...themes/teams/components/Dropdown/dropdownStyles.ts 24.13% <0%> (ø) ⬆️
...ackages/react/src/components/Dropdown/Dropdown.tsx 33.96% <24%> (+0.09%) ⬆️
packages/react/src/lib/accessibility/index.ts 100% <0%> (ø) ⬆️
packages/react/src/index.ts 100% <0%> (ø) ⬆️
packages/react/src/components/Status/Status.tsx 100% <0%> (ø) ⬆️
...rc/themes/teams/components/Icon/svg/icons/index.ts 100% <0%> (ø) ⬆️
...omponents/Icon/svg/ProcessedIcons/icons-urgent.tsx 100% <0%> (ø) ⬆️
...b/accessibility/Behaviors/Status/statusBehavior.ts
.../themes/teams/components/Icon/svg/icons/urgent.tsx
.../themes/teams/components/Icon/svg/icons/accept.tsx

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 ec4827b...dbcd0dc. Read the comment docs.

Copy link
Member

@layershifter layershifter left a comment

Choose a reason for hiding this comment

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

Only one small comment left, LGTM 👍

- introduced `open`, `defaultOpen` props
- introduced `onOpenChange` event handled
- reordered handlers and render function
@bmdalex bmdalex merged commit f979147 into master Feb 20, 2019
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.

None yet

2 participants