Portal Component #2436
Portal Component #2436
Conversation
|
|
Performance Test Results The following fails have been reported by WebpageTest. These numbers indicates a possible performance issue with the PR which requires further manual testing to validate. https://pr-2436.pwa-venia.com : LH Performance Expected 0.85 Actual 0.58, LH Best Practices Expected 1 Actual 0.92 |
| @@ -1 +1,2 @@ | |||
| export { default as Modal } from './modal'; | |||
| // To keep backwards compatibility, Modal is an alias for Portal. | |||
There was a problem hiding this comment.
Good work. Perhaps leave a deprecation notice or at least a TODO. I think Zetlen has some examples of deprecation notices in the codebase.
m2-community-project
bot
moved this from Ready for Review
to Reviewer Approved
in Pull Request Progress
jimbo
added
the
version: Minor
| export { default as Modal } from './modal'; | ||
| // For now, Modal is an alias for Portal. | ||
| // This is deprecated behavior and may be changed in the future. | ||
| export { Portal as Modal } from '../Portal'; |
There was a problem hiding this comment.
I don't think Modal was part of the last release so I don't think this would be needed.
There was a problem hiding this comment.
Unfortunately Modal has been around for a while (at least 5.0.0), so we're not sure who may be using it. I'll be sure to bring this up in community sync.
m2-community-project
bot
moved this from Reviewer Approved
to Review in Progress
in Pull Request Progress
| @@ -1 +1,3 @@ | |||
| export { default as Modal } from './modal'; | |||
| // For now, Modal is an alias for Portal. | |||
| // This is deprecated behavior and may be changed in the future. | |||
There was a problem hiding this comment.
https://jsdoc.app/tags-deprecated.html
| // This is deprecated behavior and may be changed in the future. | |
| /** | |
| * @deprecated Going forward you should use the Portal component for which Modal is now an alias. | |
| **/ |
m2-community-project
bot
moved this from Review in Progress
to Done
in Pull Request Progress
Description
This PR creates a new
Portalcomponent for rendering content into a DOM node that existsoutside of the DOM hierarchy of the parent / calling component.
The
Portalcomponent is a simple wrapper around React Portals that provides a default target if needed.We had this functionality already actually but the component was called
Modaland it lead to confusion, especially after we introduced theDialogcomponent which has a "modal" mode.In order to preserve backwards compatibility, we kept the
Modalcomponent around for now. It exists as an alias forPortal. In Venia though, all instances ofModalhave been replaced byPortal.Another option we discussed was to have the
Modalcomponent compose theDialogcomponent and set itsisModalprop totrue. This may be a future improvement and is out of scope here.Related Issue
Closes PWA-590.
Acceptance
Verification Stakeholders
@jimbo
Specification
Verification Steps
All of the places where the new
Portalcomponent is used should be checked for regressions.Please check that the following places function correctly on desktop and on mobile:
Screenshots / Screen Captures (if appropriate)
Checklist