feat: support idAliases for PluginSlots
#110
Conversation
brian-smith-tcril
force-pushed
the
id-aliases
branch
from
6e1b431 to
bdcbeb4
Compare
brian-smith-tcril
changed the title
id_aliases for PluginSlotsidAliases for PluginSlots
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #110 +/- ##
==========================================
+ Coverage 95.76% 95.91% +0.15%
==========================================
Files 10 10
Lines 236 245 +9
Branches 71 75 +4
==========================================
+ Hits 226 235 +9
Misses 9 9
Partials 1 1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
brian-smith-tcril
force-pushed
the
id-aliases
branch
3 times, most recently
from
0a3ee4e to
100128d
Compare
src/plugins/data/hooks.js
Outdated
| } | ||
| return { keepDefault: true, plugins: [] }; | ||
|
|
||
| const configSlots = allConfigSlots[lastMatchingId]; |
There was a problem hiding this comment.
Nit/question: Would this be better named as configSlot rather than the plural? Since we are only expecting the configuration for one slot?
There was a problem hiding this comment.
I agree, I was just using the name that already existed before (it was always just getting 1), and I wasn't sure if there was a specific reason for or context around the name being plural before that I was missing
There was a problem hiding this comment.
Ahh I see! Well I have no issues with the code so I have given my approval, but I think the singular makes more sense. Although I realize I don't have write access to this repo anymore so my approval is just for aesthetic purposes I suppose 😆
There was a problem hiding this comment.
It sounds like the naming being plural likely wasn't intentional in the first place, so I've updated it to be configSlot instead of configSlots (https://github.com/openedx/frontend-plugin-framework/compare/100128dd9840106b7028feb88d6559432d3f120b..2360fc186923e027aa3479dd31289476dd731005)
brian-smith-tcril
force-pushed
the
id-aliases
branch
from
100128d to
2360fc1
Compare
brian-smith-tcril
force-pushed
the
id-aliases
branch
from
2360fc1 to
c618f16
Compare
| // When defining a JS object with multiple entries | ||
| // that have the same key, only the last one is kept | ||
| // | ||
| // We want to treat all entries in the pluginSlots object |
There was a problem hiding this comment.
nit: what is "the pluginSlots object" referring to specifically? allConfigSlots?
There was a problem hiding this comment.
I think the line break made this less clear than it should be, it's "the pluginSlots object in env.config.jsx"
|
🎉 This PR is included in version 1.7.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Resolves #109
Testing PR: openedx/frontend-app-learner-dashboard#603