Add new and removed fpf slots for the Verawood release - #1504
Conversation
Co-Authored-By: Claude <noreply@anthropic.com>
|
|
||
| **Added:** | ||
|
|
||
| - `CourseInfoSlot |
There was a problem hiding this comment.
@brian-smith-tcril in the Ulmo notes, we used the fully-qualified names in these release notes, but I wasn't sure what to do here since there are two names now in frontend-base land. Which should I use? https://github.com/openedx/frontend-app-instructor-dashboard/tree/main/src/slots/CourseInfoSlot
There was a problem hiding this comment.
I'd use the slot id (org.openedx.frontend.slot.header.primaryLinks.v1)
There was a problem hiding this comment.
Actually this is kind of pointing to a bigger thing. There are slots that exist in frontend-base itself.
There was a problem hiding this comment.
Despite your second line, I should use the slot id here?
Also what URL should I use, given that I incorrectly assumed we'd use a release/verawood branch? (eg https://github.com/openedx/frontend-app-instructor-dashboard/tree/release/verawood/src/slots/CourseInfoSlot)
brian-smith-tcril
left a comment
There was a problem hiding this comment.
Overall this is looking great! Couple comments about the "Updated" section in learning. One high-level question I have is if any header slot changes were looked at. We don't have named release branches for those so the comparison would have to go based on the versions installed by each MFE (ideally the same version across the board) in each release.
| - `NotificationTraySlot | ||
| <https://github.com/openedx/frontend-app-learning/tree/release/ulmo/src/plugin-slots/NotificationTraySlot>`_ | ||
| is now `org.openedx.frontend.learning.upgrade_panel.v1 | ||
| <https://github.com/openedx/frontend-app-learning/tree/release/verawood/src/widgets/upgrade>`_ |
There was a problem hiding this comment.
The old org.openedx.frontend.learning.notification_tray.v1 still exists as an alias. So NotificationTraySlot is deprecated but still exists for this release.
| - `NotificationsDiscussionsSidebarSlot | ||
| <https://github.com/openedx/frontend-app-learning/tree/release/ulmo/src/plugin-slots/NotificationsDiscussionsSidebarSlot>`_ | ||
| is now `org.openedx.frontend.learning.right_sidebar.v1 | ||
| <https://github.com/openedx/frontend-app-learning/tree/release/verawood/src/plugin-slots/RightSidebarSlot>`_ |
There was a problem hiding this comment.
The old ids still exist as aliases.
Sidenote: not sure where right_sidebar_slot came from as an alias in there. I don't think we had it in previous releases so we shouldn't have it in Verawood. This is mostly a note-to-self to make an issue to investigate/remove it.
| - `NotificationsDiscussionsSidebarTriggerSlot | ||
| <https://github.com/openedx/frontend-app-learning/tree/release/ulmo/src/plugin-slots/NotificationsDiscussionsSidebarTriggerSlot>`_ | ||
| is now `org.openedx.frontend.learning.right_sidebar_trigger.v1 | ||
| <https://github.com/openedx/frontend-app-learning/tree/release/verawood/src/plugin-slots/RightSidebarTriggerSlot>`_ |
There was a problem hiding this comment.
Old ids still there as aliases
Same sidenote as the previous one about an extra alias we probably shouldn't have (right_sidebar_trigger_slot).
| **Removed:** | ||
|
|
||
| - `NotificationWidgetSlot <https://github.com/openedx/frontend-app-learning/tree/release/ulmo/src/plugin-slots/NotificationWidgetSlot>`_ |
There was a problem hiding this comment.
This does appear to be removed. It's not clear from https://github.com/openedx/frontend-app-learning/blob/release/verawood/docs/decisions/0010-upgrade-widget-extraction.md what a site operator that was using that slot before is supposed to do now. Not something that needs to be addressed on this page but worth adding to the ADR.
There was a problem hiding this comment.
Yeah, I dug a bit and that was the conclusion I came to
| React portal to ``document.body`` to ensure proper positioning and stacking | ||
| context. | ||
|
|
||
| **Updated:** |
There was a problem hiding this comment.
I think what I'd be looking for at a high level in this section would be:
- What IDs are deprecated
- When will the deprecated ID stop working? What should I change it to?
- Will my existing plugins still work?
- From what I can tell the shape of the slots didn't change, so existing plugins should still work.
- How do I fix it if my plugin broke? What should I do instead?
- ADR link should cover that.
There was a problem hiding this comment.
I'm not sure I have the context to fix all that :\ would you be able to make suggestions, or a PR into this branch?
|
One more thing worth mentioning: the |
I agree and disagree. I think a separate Once I have a better understanding of how to find & link to frontend-base slots (see my inline question about the instructor dash slot urls) I can do that as well. |
Made a quick script to get at this information, and then adjusted by hand. Will improve the script once I have some feedback on whether or not it did the right thing :\
https://github.com/sarina/scripts/blob/main/diff-plugin-slots.py
Co-Authored-By: Claude noreply@anthropic.com
Stack created with GitHub Stacks CLI • Give Feedback 💬