spinnaker
feat(core): added useShowModal that passes the application to the context in showModal
Problem
Some components use the ApplicationContext inside of a modal, however, it's not part of the react tree and therefore can't use the context.
Solution
Added a new parameter - application - to showModal and created a new hook that passes the application from the context to showModal
I think this problem would be better solved using react portals in showModal
I agree, I was a bit surprised that we weren't using it but didn't want to update the current implementation, because it might break some of the existing usage unintentionally. Maybe I should introduce a new hook. What do you think?
On Tue, Jun 22, 2021 at 06:07 Chris Thielen @.***> wrote:
I think this problem would be better solved using react portals in showModal
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/spinnaker/deck/pull/9363#issuecomment-865492518, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEC2OROQD5AR27QWDCV27JTTT746FANCNFSM47AI5WYA .
can you explain why adding a portal might break current uses? TBH I'm a bit surprised we arent already doing this in showModal too.
When using portal event bubbling and contexts work differently (they work 🙃). While it probably won't affect the existing modal, I can't say that for sure.
On Tue, Jun 22, 2021 at 22:51 Chris Thielen @.***> wrote:
can you explain why adding a portal might break current uses? TBH I'm a bit surprised we arent already doing this in showModal.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/spinnaker/deck/pull/9363#issuecomment-866285929, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEC2ORJVG4HL6ZBZWHGQAUTTUDST3ANCNFSM47AI5WYA .
When using portal event bubbling and contexts work differently (they work 🙃). While it probably won't affect the existing modal, I can't say that for sure.
The current workflow won't fit portal afaik. It has to be rendered as a react component and not via a function call (i.e. showModal). We might be able to add a separate workflow for that