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

fix(Popup): defaultOpen prop #659

Merged
mnajdova merged 4 commits into from
Dec 20, 2018
Merged

fix(Popup): defaultOpen prop #659

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1b3ddc8
-removed state initialization because it breaks the auto-controlled b…
Dec 20, 2018
b727f32
-update changelog
Dec 20, 2018
b232a9b
Merge branch 'master' into fix/popup-default-open-prop
Dec 20, 2018
5d5f932
Merge branch 'master' into fix/popup-default-open-prop
Dec 20, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- Fix `List` - items should be selectable @sophieH29 ([#566](https://github.com/stardust-ui/react/pull/566))
- Respect `defaultTabbable` element when `FocusZone` container gets focus @sophieH29 ([#637](https://github.com/stardust-ui/react/pull/637))
- Fix `FocusZone` - fix last breaking changes and make improvements for `Chat` usage @sophieH29 ([#614](https://github.com/stardust-ui/react/pull/614))
- Fix `defaultOpen` prop in the `Popup` component @mnajdova ([#659](https://github.com/stardust-ui/react/pull/659))

### Features
- Add `color` prop to `Text` component @Bugaa92 ([#597](https://github.com/stardust-ui/react/pull/597))
Expand Down
2 changes: 0 additions & 2 deletions src/components/Popup/Popup.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,8 +183,6 @@ export default class Popup extends AutoControlledComponent<ReactProps<PopupProps
}
}

public state = { target: undefined, open: false }

Copy link
Contributor

@sophieH29 sophieH29 Dec 20, 2018

Choose a reason for hiding this comment

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

Just to raise attention, there is an issue currently in Autocontrolled component. It doesn't allow to have both autocontrolled and non-controlled state.
Created this 1 week ago - #610

Because it might be needed to have an initial state explicitly set for target

Copy link
Contributor Author

@mnajdova mnajdova Dec 20, 2018

Choose a reason for hiding this comment

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

the target is also an auto-controlled prop in this case, so it is safe to be removed.

public componentDidMount() {
this.updateOutsideClickSubscription()

Expand Down