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

fix(Dropdown): call getDerivedStateFromProps() from AutoControlledComponent #1416

Merged
merged 8 commits into from
May 29, 2019

Conversation

layershifter
Copy link
Member

@layershifter layershifter commented May 29, 2019

Fixes #1412.

This issues is caused by inheritance of AutocontrolledComponent by Dropdown, we should its getDerivedStateFromProps() to properly compute props.

@mnajdova
Copy link
Contributor

Based on the screenshot differences it seems that we broke the multiple Dropdown variations. When something is picked in the multiple search, it should not appear in the list.

@codecov
Copy link

codecov bot commented May 29, 2019

Codecov Report

Merging #1416 into master will decrease coverage by <.01%.
The diff coverage is 80%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1416      +/-   ##
==========================================
- Coverage   73.59%   73.59%   -0.01%     
==========================================
  Files         787      787              
  Lines        5889     5888       -1     
  Branches     1737     1717      -20     
==========================================
- Hits         4334     4333       -1     
  Misses       1549     1549              
  Partials        6        6
Impacted Files Coverage Δ
...ackages/react/src/components/Dropdown/Dropdown.tsx 88.54% <80%> (-0.03%) ⬇️

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 5d3412b...0f53e56. Read the comment docs.

@layershifter layershifter added 🧰 fix Introduces fix for broken behavior. 🚀 ready for review labels May 29, 2019
static getDerivedStateFromProps(props: DropdownProps, state: DropdownState) {
static getDerivedStateFromProps(props: DropdownProps, currentState: DropdownState) {
// Due inheritance of AutoControlled component we should call its getDerivedStateFromProps(),
// to properly compute next state we should merge it with existing
Copy link
Contributor

Choose a reason for hiding this comment

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

// Due to the inheritance of the AutoControlledComponent we should call its getDerivedStateFromProps() and merge it with the existing state

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🧰 fix Introduces fix for broken behavior. 🚀 ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dropdown - Text not displayed in Input in Controlled mode
4 participants