Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add mountNode prop to Popup module #4362

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants