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

Commit 9317a80

Browse files
authored
fix: preserve outside click subscription for JS frame that processes component update (#803)
* do not miss JS frames with outside click subscription disabled * add unit tests * update changelog
1 parent 0a00d34 commit 9317a80

File tree

5 files changed

+25
-7
lines changed

5 files changed

+25
-7
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
3333
- Handle `onClick` and `onFocus` on ListItems correctly @layershifter ([#779](https://github.com/stardust-ui/react/pull/779))
3434
- Remove popup trigger button default role @jurokapsiar ([#806](https://github.com/stardust-ui/react/pull/806))
3535
- Improve `Dropdown` component styles @Bugaa92 ([#786](https://github.com/stardust-ui/react/pull/786))
36+
- Preserve outside click subscription on `Popup` and `MenuItem` component updates @kuzhelov ([#803](https://github.com/stardust-ui/react/pull/803))
3637

3738
<!--------------------------------[ v0.19.1 ]------------------------------- -->
3839
## [v0.19.1](https://github.com/stardust-ui/react/tree/v0.19.1) (2019-01-29)

src/components/Menu/MenuItem.tsx

+3-3
Original file line numberDiff line numberDiff line change
@@ -278,12 +278,12 @@ class MenuItem extends AutoControlledComponent<ReactProps<MenuItemProps>, MenuIt
278278
}
279279

280280
private updateOutsideClickSubscription() {
281-
this.outsideClickSubscription.unsubscribe()
282-
283-
if (this.props.menu && this.state.menuOpen) {
281+
if (this.props.menu && this.state.menuOpen && this.outsideClickSubscription.isEmpty) {
284282
setTimeout(() => {
285283
this.outsideClickSubscription = EventStack.subscribe('click', this.outsideClickHandler)
286284
})
285+
} else {
286+
this.outsideClickSubscription.unsubscribe()
287287
}
288288
}
289289

src/components/Popup/Popup.tsx

+3-3
Original file line numberDiff line numberDiff line change
@@ -226,9 +226,7 @@ export default class Popup extends AutoControlledComponent<ReactProps<PopupProps
226226
}
227227

228228
private updateOutsideClickSubscription() {
229-
this.outsideClickSubscription.unsubscribe()
230-
231-
if (this.state.open) {
229+
if (this.state.open && this.outsideClickSubscription.isEmpty) {
232230
setTimeout(() => {
233231
this.outsideClickSubscription = EventStack.subscribe(
234232
'click',
@@ -247,6 +245,8 @@ export default class Popup extends AutoControlledComponent<ReactProps<PopupProps
247245
},
248246
)
249247
})
248+
} else {
249+
this.outsideClickSubscription.unsubscribe()
250250
}
251251
}
252252

src/lib/eventStack/eventStack.tsx

+6-1
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,15 @@ class EventStackSubscription {
6161
return new EventStackSubscription(() => {}, true)
6262
}
6363

64-
public constructor(private _unsubscribe: Function, public readonly isEmpty: boolean = false) {}
64+
public get isEmpty(): boolean {
65+
return this._isEmpty
66+
}
67+
68+
public constructor(private _unsubscribe: Function, private _isEmpty: boolean = false) {}
6569

6670
public unsubscribe(): void {
6771
this._unsubscribe()
72+
this._isEmpty = true
6873
}
6974
}
7075

test/specs/lib/eventStack/eventStack-test.ts

+12
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,11 @@ import { domEvent } from 'test/utils'
33

44
describe('eventStack', () => {
55
describe('sub', () => {
6+
test('makes subscription to be non-empty', () => {
7+
const clickSubscription = EventStack.subscribe('click', jest.fn())
8+
expect(clickSubscription.isEmpty).toBe(false)
9+
})
10+
611
test('subscribes for single target', () => {
712
const handler = jest.fn()
813

@@ -47,6 +52,13 @@ describe('eventStack', () => {
4752
})
4853

4954
describe('unsub', () => {
55+
test('makes subscription to be empty', () => {
56+
const clickSubscription = EventStack.subscribe('click', jest.fn())
57+
clickSubscription.unsubscribe()
58+
59+
expect(clickSubscription.isEmpty).toBe(true)
60+
})
61+
5062
test('unsubscribes and destroys eventTarget if it is empty', () => {
5163
const handler = jest.fn()
5264

0 commit comments

Comments
 (0)