elastic / eui

Elastic UI Framework 🙌
https://eui.elastic.co/
Other
6.08k stars 826 forks source link

EuiPortal should be able to use different portal target than document.body based on provided Context #7778

Open prcdpr opened 3 months ago

prcdpr commented 3 months ago

Is your feature request related to a problem? Please describe. I'm trying to render EuiFlyouts inside the child window opened via window.open. Most of the things work except for Flyouts/Modals as they open in the parent window instead of child window.

I use React portals to render into the child window (via my own <ChildWindow> component).

Describe the solution you'd like

I'd like to do something like the following (pseudocode):

<ChildWindow>
  {(childWindow) => (
     <EuiDomWindowContextProvider target={childWindow.document}>

       <EuiFlyout>....</EuiFlyout>

     </EuiDomWindowContextProvider>

  )
</ChildWindow>

EuiDomWindowContextProvider would be a new component that provides window handle via React context to its children.

Existing type EuiPortal, instead of using always the document.body target as in https://github.com/elastic/eui/blob/main/packages/eui/src/components/portal/portal.tsx#L77 it would try to resolve the portal target using the provided value from EuiDomWindowContextProvider first.

As fallback (no context provided) it would use document.body. This would make the change non-breaking.

Describe alternatives you've considered The only possible workaround right now, would be to disable ownFocus on EuiFlyout and wrap each flyout with custom portals and overlay masks.

Desired timeline This isn't the most frequent use case for most people but at the same moment is very easy to implement. It would unblock me with using EUI in multi-window Electron app.

prcdpr commented 3 months ago

EuiWindowEvent would need same treatment too: https://github.com/elastic/eui/blob/main/packages/eui/src/services/window_event/window_event.ts

It relies on hardcoded window object making it incompatible when rendering via React Portals to child windows.

Meaning that only possible workaround for now is to fork the EUI and apply patches.

cee-chen commented 3 months ago

Is this something you could resolve via EuiProvider's componentDefaults.EuiPortal.insert API? This prop allows you to configure the insert location of all EuiPortals across the board, including the ones used in flyouts.

https://eui.elastic.co/#/utilities/provider#component-defaults

Regarding EuiWindowEvent, we'd consider taking a componentDefaults configuration for that as well, but it would likely be low priority / we'd want to receive a PR for it.

stil commented 3 months ago

componentDefaults.EuiPortal.insert wouldn't work, as per documentation it's global, not scoped to the component tree.

Unless it supports nested EuiProvider tags.

Perhaps I'll create a PR for my proposal if I manage to get it working.

(I'm the original issue author just using my private account)

cee-chen commented 3 months ago

Ah gotcha, no, nested EuiProviders are not supported, sorry - I hadn't realized you were trying to configure this for just one or two flyouts. A PR would be much appreciated!

Edit: To clarify, based on the complexity of the PR, we may not accept it as this is very edge case for Elastic's use cases, but we would certainly review it. If it were a small, elegant, and safe enough change, that would certainly weigh us towards accepting it!

prcdpr commented 3 months ago

What's the recommended way of using locally forked EUI repository?

Currently my flow is:

On the client repo:

It works flawlessly however the yarn build step is quite long and it takes ~3 minutes. Do you have any guidelines how to consume forked EUI but with faster iterations?

tkajtoch commented 3 months ago

Hi @prcdpr! We recommend yarn link-ing your local EUI fork to other projects (see yarn docs).

There isn't a built-in script to build EUI in watch mode as a library (updating the es and lib directories whenever the src directory contents change). You should, however, be able to run one of the following commands to skip the 3-minute build time every time you make a simple change:

Please note that these commands should be considered a quick and dirty fix to reduce recompilation/prevew time, and you should always run yarn build afterward to emit all of the necessary files.

prcdpr commented 3 months ago

Watch recompilation commands work however I didn't manage to get it working with yarn link due to local EUI repo having inconsistent node_modules with consuming app causing tons of TS/React errors.

yalc publish && yalc push in combination with yarn run babel --watch works though, even though I need to publish/push manually but at least I don't have to wait 3 minutes for full build.