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

feat: add direct plugin framework #16

Merged
merged 12 commits into from
Jan 18, 2024

Conversation

jsnwesson
Copy link
Contributor

@jsnwesson jsnwesson commented Jan 9, 2024

APER-3108
This PR formally adds Braden McDonald's plugin framework as a non-iFrame alternative for the library. The logic is still separated from the original framework's handling, so a follow-up ticket will be to integrate the two plugins -- iFrame and Direct.

To see this PR running in Learner Dashboard:

  1. Pull branches down from Learner Dashboard
  2. Checkout aperture/add-UI-Plugin-Framework
  3. npm install and npm run start
  4. The files with the Direct Plugin logic will be found in /src/uiPlugins/SideBarSlot/index.jsx, src/uiPlugins/DemoPlugin/index.jsx, and src/containers/Dashboard/index.jsx
    Changes can also be found here in GitHub

Copy link

codecov bot commented Jan 10, 2024

Codecov Report

Attention: 14 lines in your changes are missing coverage. Please review.

Comparison is base (3b9139d) 75.19% compared to head (7b0326e) 72.61%.
Report is 2 commits behind head on master.

Files Patch % Lines
src/plugins/directPlugins/DirectPluginSlot.jsx 0.00% 14 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master      #16      +/-   ##
==========================================
- Coverage   75.19%   72.61%   -2.58%     
==========================================
  Files          10       13       +3     
  Lines         129      168      +39     
  Branches       19       30      +11     
==========================================
+ Hits           97      122      +25     
- Misses         30       44      +14     
  Partials        2        2              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

* added some further documentation to explain what the configuration of allSlotChanges and slotChanges should look like
* changed variable naming from original source code to better define data objects
* changed assumption of what is returned by DirectPluginContext to be a single object and not array

/**
This is what the allSlotChanges configuration should look like when passed into DirectPluginContext
{
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The one thing that I decided to change in order to steer away from Braden's approach and move closer to eventually using a JS-based config is that his configuration assumed that there'd be multiple allSlotChanges for a Host MFE, and I think whatever benefit that had could've eventually led to confusion/oversight.
I'd argue that keeping all known changes in this one object is easier to track, especially as they'll ideally live in a single JS config anyways.

Copy link
Member

@MaxFrank13 MaxFrank13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for putting this together. It looks great so far. I just had some questions to clear up things that were still hazy to me and a couple recommendations that were pretty minor.

@jsnwesson jsnwesson merged commit 66df89e into master Jan 18, 2024
5 of 7 checks passed
@jsnwesson jsnwesson deleted the jwesson/add-direct-plugin-framework branch January 18, 2024 16:23
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