Skip to content

Conversation

svicalifornia
Copy link

If a React app with SUIR is running in a hidden IFRAME and rendering some or all of its virtual DOM to the parent frame or another frame, then the Popup component should not insert popup elements into the document.body of the React IFRAME, but instead should inject into the frame containing the Popup trigger, or even into some other container as may be more appropriate (due to size or layering concerns between multiple frames).

Adding a mountNode prop to Popup allows developers to specify the appropriate container for a displayed popup element in cases such as the above example.

This PR implements a mountNode prop for Popup, with code very similar to the same prop's implementation in Modal.

If a React app with SUIR is running in a hidden IFRAME and rendering some or all of its virtual DOM to the parent frame or another frame, then the Popup component should not insert popup elements into the document.body of the React IFRAME, but instead should inject into the frame containing the Popup trigger, or even into some other container as may be more appropriate (due to size or layering concerns between multiple frames). Adding a `mountNode` prop to Popup allows developers to specify the appropriate container for a displayed popup element in cases such as the above example.
@welcome
Copy link

welcome bot commented Apr 29, 2022

💖 Thanks for opening this pull request! 💖

Here is a list of things that will help get it across the finish line:

  • Run yarn lint locally to catch formatting errors. This will fix some errors automatically, commit and push any changes.
  • Run yarn test locally to catch errors. This ensures all components still behave as they should.
  • Run yarn start to run the doc site locally and try a few pages, ensuring everything is in good working order.
  • Include tests when adding/changing behavior.

We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can.

@codecov-commenter
Copy link

codecov-commenter commented Apr 29, 2022

Codecov Report

Merging #4362 (7832ab6) into master (49f0056) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@ Coverage Diff @@ ## master #4362 +/- ## ======================================= Coverage 99.75% 99.75% ======================================= Files 180 180 Lines 3248 3250 +2 ======================================= + Hits 3240 3242 +2  Misses 8 8 
Impacted Files Coverage Δ
src/modules/Popup/Popup.js 98.95% <100.00%> (+0.02%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 49f0056...7832ab6. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants