Adding the header slot to discussions MFE?

It seems that the discussions MFE does not support the header plugin slot, which makes the discussions page much different from the other pages.

Is this something that can be added easily by forking the MFE ?

I was surprised to hear this, but it’s true. I opened a bug. Thanks for reporting it!

Whether it’s easy or not depends a little on skill, but in any case, I suggest copying what the Learning MFE does.

Thanks. I am happy to test it once it’s there.

I upgraded to Verawood, with tutor and tutor-forum 22.0.0, but I still don’t see the correct header in the discussions page of a course.

Is there anything more that is needed ?

The slot is there. Are you wiring it up according to its documentation?

Unless there is something different between discussions and learner or other MFEs, I apply the header slot changes to all MFEs in here:

https://github.com/calculquebec/tutor-plugins/blob/cq/verawood.dev/plugin_slots.py

I’ll try to debug

Ok, by comparing the header slot in the learner vs discussions MFE, through the plugin slot browser:

https://arunmozhi.in/openedx-plugin-slots-browser/mfes/frontend-app-discussions/slots/HeaderSlot/

vs

https://arunmozhi.in/openedx-plugin-slots-browser/mfes/frontend-app-learning/slots/HeaderSlot/

I believe the issue I had is that for the discussions app, the plugin slot

org.openedx.frontend.layout.header_discussions.v1

does not have an alias named header_slot

like the other MFEs.

That threw off the logic of my plugin slots, because I was using the alias. Whether this is intended or a bug, I don’t know. I find aliases useful because they reduce the amount of code needed to maintain, but I understand that they are less specific.

After adding another plugin slot specifically for

org.openedx.frontend.layout.header_discussions.v1

then the header is replaced by ours like in other pages.

@brian.smith, should it? I see Maxime’s point, but as I remember it, we only ever intended aliases to be used for backward-compatibility, which is not technically applicable, here.

Aliases are intended for backwards compatibility, yes.

This is an interesting case.

frontend-app-learning used header_slot for org.openedx.frontend.layout.header_learning.v1 before we moved to reverse DNS naming.

frontend-app-learner-dashboard, frontend-app-profile, frontend-app-account etc. don’t have specific header_slots and instead rely on the slots provided by frontend-component-header:

It seems like the desired functionality would best be achieved by having a shared slot for the LearningHeader component in frontend-component-header. Following the desktop/mobile pattern the naming would be org.openedx.frontend.layout.header_learning.v*, so we’d probably need to use v2 to not overlap with the existing slot in frontend-app-learning.

We’d then want to “slot in a slot” it, keep the existing slots in frontend-app-learning and frontend-app-discussions so people can continue to target each individually, or choose to use v2 to target both at once.

With that in place, adding header_slot as an alias for the new org.openedx.frontend.layout.header_learning.v2 would be a bad idea, as it would be an alias shared between 2 slots in the same frontend-app-learning render tree.

A path I would wholeheartedly agree with if we were not already solving the general use case with frontend-base and the unified shell header. In other words, I feel we should leave this alone and focus on getting all the learner-facing MFEs that would benefit from this converted to frontend-base by Willow… which we happen to be doing already. :wink: