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

The subscription for EventStack should be refactored in the Popup and MenuItem components #649

Closed
mnajdova opened this issue Dec 19, 2018 · 0 comments
Labels
vsts Paired with ticket in vsts

Comments

@mnajdova
Copy link
Contributor

Bug Report

@kuzhelov: The implementation used in the Popup and the MenuItem for subscribing to EventStack has one subtle problem: although it solves the issue of item being opened and closed within the same frame, it also opens possibility for the following behavior:

MenuItem has opened submenu, this opened prop value doesn't change anyhow for the next JS frame.
///------ JS FRAME --------////

render method was called (but submenuOpen wasn't changed changed anyhow, it is still true)

updateOutsideClickSubscription

this.outsideClickSubscription.unsubscribe(), no subscription on outside click (!!)
setTimeout(() => subscription will be made on the next frame)
... some other logic ..

note, there is no click subscription at this moment, but it should be (!!)
////---------------------------////

Would suggest to leave a fix for a separate PR, for both components.

Reference: #539 (comment)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
vsts Paired with ticket in vsts
Projects
None yet
Development

No branches or pull requests

2 participants