Change request: switch to the frontend-component-header
dyudyunov opened this issue · 13 comments
The problem
Instead of using the frontend-component-header the MFE uses its own LearnerDashboardHeader component. Main pain points here:
- it’s a third header on the platform (including the legacy header and the header component used by other MFEs) which is confusing for users
- it has hardcoded edx’s values, like the edx logo, which I believe will be a violation of the trademark policies if used elsewhere
Proposal
It would be great to replace it with the easily-brandable frontend-component-header.
Additional info
[inform] Just dropping some helpful links for whenever this issue gets picked up:
- Hardcoded edX.org production logo:
- Ideally, instead of hardcoding the logo URL as shown above, it'd rely on the configured
LOGO_URL, e.g.getConfig().LOGO_URL:
This was partially solved by: #205, which makes it customizable by adding an environment variable. Which value is read during build time. So we would need to add support for the MFE Config for a more flexible solution as Adam mentioned. Although I'm wondering why we are using a custom header instead of the LearningHeader -- or a similar version as suggested in this issue? Which sounds like the best long-term solution but with lots of work.
Here's a PR with the suggested change (getting the logo from the MFE API): #238
Hey all, just wanted to jump in to say that my team (@openedx/2u-aperture, soon-to-be-maintainers) and I fully support replacing the header with the default MFE Header. We'll definitely want to know when that change is ready to happen, but as a heads up we'd want to ensure two things in order to accept the PR:
- The "Careers" option still appears for edX.org users (original PR with screenshots, fair warning that there is a bug somewhere that is hiding this)
- The learner will still be able to toggle between their dashboard or one of the available Enterprise dashboards via the modal or dropdown.
@jsnwesson: thanks for the details! I don't know how much work it would be to change the header, but when we have it done, is it enough to have flags to turn on/off both dashboards? Unless we have a replacement for the community, I'll tag @arbrandes here. Should we talk with the product wg for this one?
@mariajgrimaldi I would say it's enough to have flags for the dual dashboard feature!
I'm not involved as much with the argument for certain Header implementations over others, but I would imagine that if we ever want to make it so the Header and Footer can be wrapped around all MFEs (Piral being one method that comes to mind), then we'd want to stick with the same Header/Footer dependencies across the board.
Hi everyone, Just want to know if anyone is actively working on this at the moment? or Are we paused because of "Careers" option and dual dashboard thing.
What if we'll include both things in @edx/frontend-component-header and use that header package in all MFEs for UI consistency or is there any problem in that? Looking forward to your thoughts on this so that I can create PR.
Hi @hinakhadim ! As it turns out, I believe the @openedx/2u-vanguards team is planning on bringing in the customized header into @edx/frontend-component-header. @zainab-amir is that still accurate?
Hi @zainab-amir and @openedx/2u-vanguards, just wanted to know whether what @jsnwesson wrote above is correct and if anyone is working on this at the moment? One of our clients is interested in making the header look consistent across all MFEs, so depending on what needs to be done, we could probably create a PR as well or help with an existing one if needed. Looking forward to your thoughts.
That's awesome, @syedsajjadkazmii! Thank you and your team for working on this. Please let us know if we can help anyhow.
@arbrandes yes, that is the PR to replace the learner dashboard custom header with frontend-component-header. We have a small followup PR that needs your review: openedx/frontend-component-header#494