-
Notifications
You must be signed in to change notification settings - Fork 25
refactor: revert default content to live in host MFE #41
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
Conversation
* Default content should not be defined in the JS config * keepDefault boolean in JS config will determine whether content is rendered
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #41 +/- ##
==========================================
+ Coverage 94.57% 96.53% +1.95%
==========================================
Files 10 10
Lines 166 173 +7
Branches 32 35 +3
==========================================
+ Hits 157 167 +10
+ Misses 7 5 -2
+ Partials 2 1 -1 ☔ View full report in Codecov by Sentry. |
MaxFrank13
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great. Thanks for getting all this done. I just had a handful of nit picks. I'm going to approve now but am happy to discuss anything further 👍
* the original test was going to hide defaultContent, but that is already accomplished with keepDefault functionality
This PR extracts the default component logic from
env.config.jsto live in the Host MFE, and thus wrapped by the Plugin Slot. This is necessary in order to ensure that core functionality remains in the Host MFE rather than defined in a JS config, which would have impacted the developer experience for engineers who weren't planning on using the plugin slot.The primary changes include:
keepDefaultboolean replacesdefault_contentsarray in JS configkeepDefaultis set tofalseAPER-3281