I’ve looked into customising the branding for the MFEs and right now there are two paths:
Override Logo URLs variables in the environment.
Override @edx/brand package.
Logo URLs
At the moment, we have to override the values using patches (mfe-env-production, mfe-env-development) and to override LOGO_URL, we have to add a variable LOGO_URL using the patch.
Would it be more convenient to have a variable that can be set in the config file? The definition of the variable in the plugin would become
LOGO_URL='{% if MFE_LOGO_URL is defined %}{{ MFE_LOGO_URL }}{% else %}{{ "https" if ENABLE_HTTPS else "http" }}://{{ LMS_HOST }}/static/images/logo.png{% endif %}'
I don’t think there is anyway right now in tutor to apply the branding package to all MFEs. I propose we add the following patch to the dockerfile (see PR).
ARG NPM_REGISTRY=https://registry.npmjs.org/
{{ patch("mfe-dockerfile-npm-overrides") }}
RUN npm install --verbose --registry=$NPM_REGISTRY
With this patch, the following plugin overrides the brand package:
Sorry for the late reply :-/ Here’s my opinion on these two items.
Logo URLs
It makes total sense that operators should be able to easily override MFE branding urls. However, I’d rather avoid introducing many additional plugin settings. Instead, our approach so far has always been to:
Provide good defaults.
Make it possible to override these defaults with the help of plugins.
In the case of MFE branding urls, I propose the following:
Define good default branding urls that point to the LMS favicon, logo, etc. both in development and production.
Describe in the tutor-mfe README how to make use of the “mfe-env-production” and “mfe-env-development” patches to override these values.
Would that make sense to you?
Overriding @edx/brand package
Your proposal makes total sense (again). I would only propose a different patch name: “mfe-dockerfile-pre-npm-install”. Also, we should take the opportunity to add “mfe-dockerfile-post-npm-install” to install requirements after the first npm install command is run.
Yes, it makes sense. It did feel like a lot of variables. Also documenting the process will clarify it.
I’ll modify my PR to include these two points.
Overriding @edx/brand package
Happy to change the name but in that case, would it also make sense to add documentation to the Readme? I set the name as it is so that the intent is obvious. Though I understand it’s not “just” for overrides.
For the Readme, I also want to add screenshots of the resulting customised MFE. Can you create a fork of brand-openedx (brand-overhangio?)?
I’m not sure what this new repo is for? Do we want to provide a good usable theme out of the box that is different from brand-openedx? I think that’s a great idea, in the same vein as providing the indigo theme. But will it be very different from the brand-openedx package? Shouldn’t we make changes to the upstream theme instead?
Hi @regis and @BbrSofiane! I followed this thread and I was able to install custom branding and components to MFEs with a patch to mfe-dockerfile-post-npm-install. Thanks for making that possible!
However, I am wondering what you would think about adding configuration values for the same?
For example, adding something like follows to the mfe Dockerfile:
{% for key, value in MFE_NPM_CONFIG_EXTRA.items() %}
RUN npm config set '{{ key }}' '{{ value }}'
{% endfor %}
{% for item in MFE_NPM_OVERRIDES %}
RUN npm install '{{ item }}'
{% endfor %}
So, one could use MFE_NPM_CONFIG_EXTRA to define a private registry and token for custom components and MFE_NPM_OVERRIDES to install said components and branding.
A config.yml example:
I’d say the gain would be one less yaml plugin (which at least in our case would have to be additionally configurable) and with added docs this could be quite user friendly. Of course, I might be missing something that would make this a bad idea
Would a PR for such a change be welcomed or should we stick to using a yaml plugin?
So right now to customise the branding you have a plugin like this
name: mfe-branding
version: 0.1.0
patches:
mfe-dockerfile-post-npm-install: |
RUN npm config set '//gitlab.com/api/v4/projects/<project_id>/packages/npm/:_authToken' <token>
RUN npm config set @foo:registry https://gitlab.com/api/v4/projects/<project_id>/packages/npm/
RUN npm install @edx/brand@git+https://<brand_repo>'
RUN npm instal @edx/frontend-component-footer@npm:@foo/<custom_footer_component>@<version>
and instead you’re suggesting we add config parameters to replace the above plugin by
Personally, I have 1 plugin that has all the patches specific to my instance so I don’t really view this as a problem. But I appreciate that depending on the scale of your deployment it might not be that simple so my opinion on this might be skewed .
I see that having configuration parameters like MFE_NPM_CONFIG_EXTRA and MFE_NPM_OVERRIDES makes the intent more explicit and gives users a clear path. I guess the downside is that it also narrows the scope of the customisation.
Ideally, we don’t end up creating specific configuration values for specific types of customisation but instead provide the entry points for users to do what they need.
Do you picture having both the parameters you suggest and the patch mfe-dockerfile-post-npm-install?
I’m leaning towards not adding those extra configuration settings but I think having a tutorial for “How to use components from a private registry” could be a good way to document the process.
.