eui

eui copied to clipboard

Summary

Closes #7778

The main idea in this PR is to enable multi-windowed React development with EUI, for example in Electron. The change is purely bug-fixing and does not introduce any UI changes.

I want to enable multi-windowed support by using React Portals to render into different DOM windows. The problem is that, in many places, EUI operates on global window or document object which I aim to fix.

It's not a completely novel idea, something similar can be found in FluentUI: https://github.com/microsoft/fluentui/tree/master/packages/react-window-provider

New <EuiWindowProvider> component is exposing current window object for its children elements.

I'm marking it as draft so early-on you can have a look what's inside and perhaps immediately tell that it's not going to be merged, then I'll continue work internally.

The change is completely backwards compatible and does not introduce any behavior or UI changes.

Remaining work:

• [ ] See why outside click does not fire in child windows (https://github.com/theKashey/react-focus-on/issues/88).
• [ ] Do additional testing of different kind of behaviors in my client application.

The most important is that with these changes Flyouts work in child windows!

Let me know wheter this is something you'd consider at all for merging (assuming that all potential issues with PR are fixed).

QA

General checklist

• Browser QA
  • [ ] ~~Checked in both light and dark modes~~
  • [ ] ~~Checked in mobile~~
  • [ ] ~~Checked in Chrome, Safari, Edge, and Firefox~~
  • [ ] ~~Checked for accessibility including keyboard-only and screenreader modes~~
• Docs site QA
  • [ ] Added documentation
  • [ ] Props have proper autodocs (using @default if default values are missing) and playground toggles
  • [ ] ~~Checked Code Sandbox works for any docs examples~~
• Code quality checklist
  • [ ] Added or updated jest and cypress tests
• Release checklist
  • [ ] A changelog entry exists and is marked appropriately.
  • [ ] ~~If applicable, added the breaking change issue label (and filled out the breaking change checklist)~~
• Designer checklist
  • [ ] ~~Updated the Figma library counterpart~~

💚 CLA has been signed

👋 Since this is a community submitted pull request, a Buildkite build has not been started automatically. Would an Elastic organization member please verify the contents of this pull request and kick off a build manually?

👋 Hey there. This PR hasn't had any activity for 90 days. We'll automatically close it if that trend continues for another week. If you feel this issue is still valid and needs attention please let us know with a comment.

❌ We're automatically closing this PR due to lack of activity. Please comment if you feel this was done in error.

Please reopen - I'm still working on fixes and now it's in better state than before with many bugs fixed.

Re-opening! Apologies @stil, we hadn't even realized this PR was in the queue - we don't get automatically pinged/notified for draft PRs unless they open.

👋 Since this is a community submitted pull request, a Buildkite build has not been started automatically. Would an Elastic organization member please verify the contents of this pull request and kick off a build manually?

buildkite test this

As a heads up you will need to sign the Elastic CLA with the email address associated with the git commits (use git log to see that email address).

:green_heart: Build Succeeded

• Buildkite Build
• Commit: 3b73f6c013196df7d5bb9aa52930bb0b458496e8

@stil Any update on signing https://www.elastic.co/contributor-agreement? We cannot move forward with merging this in without it signed with the emails associated with the commits in this PR, unfortunately.

Sorry I thought you receive a notification when signed.

I have already signed the agreement.

I'll complete rest of the steps by the end of week and mark it for review.

Current status:

• We're still waiting for my 2 MR's on third-party libraries to be accepted.
  • https://github.com/theKashey/react-focus-on/pull/96
  • https://github.com/theKashey/react-style-singleton/pull/12
  • I have tested documentation page using my custom builds of the libraries attached with yalc.
• I've updated the checklist in original post.
  • Added changelog entry.
  • Added docs page with working examples of EuiFlyout, EuiPopover and EuiTooltip inside popup window.
  • No tests yet - I'm not that familiar with the testing library and I'm unsure what testing scenarios would you need, I'd be glad to have some guidance here.
  • CodeSandbox example is not working yet due to 3rd party libs waiting for fixes.

My suggestion would be to wait a bit for the 3rd party libs MRs to be merged and released, without these we cannot really merge this MR as the behavior is a bit buggy.

When third party libs are fixed, I'm going to:

• Update the changelog entry with updated package.json dependency on react-focus-on.
• Ensure that merged bugfix in react-style-singleton works out of the box with EUI. The fix might be either via exposed WindowProvider from react-style-singleton or via internal window object passing in which case no modifications are going to be done. This is still being worked on by react-style-singleton author.

Anyway in the current state, the fixes for EUI, react-focus-on and react-style-singleton work great in my usecase even though it was a bit painful to track bugs across the libraries 😅

👋 Hey there. This PR hasn't had any activity for 90 days. We'll automatically close it if that trend continues for another week. If you feel this issue is still valid and needs attention please let us know with a comment.

❌ We're automatically closing this PR due to lack of activity. Please comment if you feel this was done in error.