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

fix: preserve outside click subscription for JS frame that processes component update #803

Merged
merged 6 commits into from
Jan 31, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- Handle `onClick` and `onFocus` on ListItems correctly @layershifter ([#779](https://github.com/stardust-ui/react/pull/779))
- Remove popup trigger button default role @jurokapsiar ([#806](https://github.com/stardust-ui/react/pull/806))
- Improve `Dropdown` component styles @Bugaa92 ([#786](https://github.com/stardust-ui/react/pull/786))
- Preserve outside click subscription on `Popup` and `MenuItem` component updates @kuzhelov ([#803](https://github.com/stardust-ui/react/pull/803))

<!--------------------------------[ v0.19.1 ]------------------------------- -->
## [v0.19.1](https://github.com/stardust-ui/react/tree/v0.19.1) (2019-01-29)
Expand Down
6 changes: 3 additions & 3 deletions src/components/Menu/MenuItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -278,12 +278,12 @@ class MenuItem extends AutoControlledComponent<ReactProps<MenuItemProps>, MenuIt
}

private updateOutsideClickSubscription() {
this.outsideClickSubscription.unsubscribe()

if (this.props.menu && this.state.menuOpen) {
if (this.props.menu && this.state.menuOpen && this.outsideClickSubscription.isEmpty) {
setTimeout(() => {
this.outsideClickSubscription = EventStack.subscribe('click', this.outsideClickHandler)
})
} else {
this.outsideClickSubscription.unsubscribe()
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/components/Popup/Popup.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -226,9 +226,7 @@ export default class Popup extends AutoControlledComponent<ReactProps<PopupProps
}

private updateOutsideClickSubscription() {
this.outsideClickSubscription.unsubscribe()

if (this.state.open) {
if (this.state.open && this.outsideClickSubscription.isEmpty) {
setTimeout(() => {
this.outsideClickSubscription = EventStack.subscribe(
'click',
Expand All @@ -247,6 +245,8 @@ export default class Popup extends AutoControlledComponent<ReactProps<PopupProps
},
)
})
} else {
this.outsideClickSubscription.unsubscribe()
}
}

Expand Down
7 changes: 6 additions & 1 deletion src/lib/eventStack/eventStack.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,15 @@ class EventStackSubscription {
return new EventStackSubscription(() => {}, true)
}

public constructor(private _unsubscribe: Function, public readonly isEmpty: boolean = false) {}
public get isEmpty(): boolean {
return this._isEmpty
}

public constructor(private _unsubscribe: Function, private _isEmpty: boolean = false) {}

public unsubscribe(): void {
this._unsubscribe()
this._isEmpty = true
}
}

Expand Down
12 changes: 12 additions & 0 deletions test/specs/lib/eventStack/eventStack-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@ import { domEvent } from 'test/utils'

describe('eventStack', () => {
describe('sub', () => {
test('makes subscription to be non-empty', () => {
const clickSubscription = EventStack.subscribe('click', jest.fn())
expect(clickSubscription.isEmpty).toBe(false)
})

test('subscribes for single target', () => {
const handler = jest.fn()

Expand Down Expand Up @@ -47,6 +52,13 @@ describe('eventStack', () => {
})

describe('unsub', () => {
test('makes subscription to be empty', () => {
const clickSubscription = EventStack.subscribe('click', jest.fn())
clickSubscription.unsubscribe()

expect(clickSubscription.isEmpty).toBe(true)
})

test('unsubscribes and destroys eventTarget if it is empty', () => {
const handler = jest.fn()

Expand Down