Semantic-ui-react: Modal: `keydown` event propagates to all modals

[mic4ael]

Bug Report

Steps

1. Create a Modal trigger which opens a Modal on click.
2. Put another trigger element inside the first Modal which opens another Modal on click.
3. Press Escape.

Expected Result

The top modal is being closed whereas the second one stays open.

Actual Result

• All modals are being closed
• Clicking outside the Modal closes only the visible Modal

Version

0.87.3

Testcase

https://codesandbox.io/embed/semantic-ui-react-yenf7

[layershifter]

@mic4ael this will be fixed in next release because root cause of these issues are same as #3194, #3174.

We tested a new approach under Stardust UI with @stardust-ui/react-component-nesting-registry and it works perfectly for a while. However, it will require [email protected], be ready for upgrade 🚀

[layershifter]

You can check an example here: https://stardust-ui.github.io/react/components/popup#usage-nested
It handles any level of nesting correctly 😋

[mic4ael]

Can you point me to a specific commit that fixes this issue? I am curious what causes this problem.

[mic4ael]

At leat I want to understand why this happens ;)

[layershifter]

Can you point me to a specific commit that fixes this issue? I am curious what causes this problem.

At SUIR, we are using EventStack to handle events in nested components.

However, this abstraction is not working perfectly in the real world. It tries do too much: handle nesting in React/DOM trees via events, it was a mistake. This idea is not working at all (_it works differently than browser's event loop_) and it creates event glitches which can't be debugged easily.

---

Solution was to separate React/DOM nesting and event handling.

• Events are handled via dumb component like react-event-listener, so there are no any issues with event loop any more as it's a regular document.addEventListener().
• React nesting is handled via solution similar to react-register-nodes, we registering all DOM nodes and then can easily iterate via them.

See https://github.com/stardust-ui/react/pull/1075 and https://github.com/stardust-ui/react/pull/949, the code is not obvious and quite complicated 💥

Key thing there is that you can register your DOM nodes and and then call getRefs() to receive all refs for current and below levels. Events are handled transparently and for Escape keys only stopPropagation is used and it's an expected behavior.

[xumix]

@layershifter Hi! Do you shift your efforts to stardust-ui from Semantic and Fomantic?

[layershifter]

@xumix it's _true_ and _not true_.

Stardust UI is my official job and there is full time team to support it at Microsoft, _short answer_.

We introduced a lot of new ideas at Stardust UI and many of them will be backported to Semantic UI React. And you can see it now, I introduced new parts of documentation and knobs in SUIR 🎉 Actually, latest updates in Popup with moving to react-popper were originally done in Stardust.

I will continue to share code between SUIR and Stardust. Actually, to fix this issue I am going to use packages from @stardust namespace 😼 At Stardust we are working on new hooks APIs that will allow us to share more code and get improvements on SUIR side in state management and accessibility.

SUIR development is not stopped and will be continued. I personally focused on fixing bugs and docs improvements. I am filling that SUIR is mature enough and we are going to make 1.0 this year finally.

[xumix]

@layershifter Ok, thanks for your dedication :)