Set edx.org theme as default for edx-platform and MFEs #67
Conversation
mfarhan943
force-pushed
the
farhan/default-edx-theme
branch
from
361136b to
b2d1484
Compare
|
@mfarhan943 are we waiting for a rereview from @adamstankiewicz or something else on this PR? |
|
@dianakhuang Yes, we are waiting for @adamstankiewicz review. |
docker-compose.yml
Outdated
| @@ -873,6 +895,8 @@ services: | |||
| service: microfrontend | |||
| working_dir: '/edx/app/frontend-app-program-console' | |||
| container_name: "edx.${COMPOSE_PROJECT_NAME:-devstack}.frontend-app-program-console" | |||
| environment: | |||
| PARAGON_BRAND_PACKAGE: '@edx/brand-edx.org' | |||
There was a problem hiding this comment.
nit: @edx/brand-edx.org@2
There was a problem hiding this comment.
The update file contains the value @edx/brand-edx.org@1.x, which I updated from this source.
docker-compose.yml
Outdated
| @@ -707,6 +707,8 @@ services: | |||
| service: microfrontend | |||
| working_dir: '/edx/app/frontend-app-account' | |||
| container_name: "edx.${COMPOSE_PROJECT_NAME:-devstack}.frontend-app-account" | |||
| environment: | |||
| PARAGON_BRAND_PACKAGE: '@edx/brand-edx.org@@1.x' | |||
There was a problem hiding this comment.
| PARAGON_BRAND_PACKAGE: '@edx/brand-edx.org@@1.x' | |
| PARAGON_BRAND_PACKAGE: '@edx/brand-edx.org@1.x' |
docker-compose.yml
Outdated
| @@ -722,6 +724,8 @@ services: | |||
| service: microfrontend | |||
| working_dir: '/edx/app/frontend-app-profile' | |||
| container_name: "edx.${COMPOSE_PROJECT_NAME:-devstack}.frontend-app-profile" | |||
| environment: | |||
| PARAGON_BRAND_PACKAGE: '@edx/brand-edx.org' | |||
There was a problem hiding this comment.
As is, this will pull from latest, which could introducing breaking changes unintentionally if a new major version is released.
| PARAGON_BRAND_PACKAGE: '@edx/brand-edx.org' | |
| PARAGON_BRAND_PACKAGE: '@edx/brand-edx.org@~2.0.0' |
docker-compose.yml
Outdated
| @@ -737,6 +741,8 @@ services: | |||
| service: microfrontend | |||
| working_dir: '/edx/app/frontend-app-authn' | |||
| container_name: "edx.${COMPOSE_PROJECT_NAME:-devstack}.frontend-app-authn" | |||
| environment: | |||
| PARAGON_BRAND_PACKAGE: '@edx/brand-edx.org@~2.0.0' | |||
There was a problem hiding this comment.
edx-internal shows this version as 2.x currently.
| PARAGON_BRAND_PACKAGE: '@edx/brand-edx.org@~2.0.0' | |
| PARAGON_BRAND_PACKAGE: '@edx/brand-edx.org@2.x' |
docker-compose.yml
Outdated
| @@ -752,6 +758,8 @@ services: | |||
| service: microfrontend | |||
| working_dir: '/edx/app/frontend-app-course-authoring' | |||
| container_name: "edx.${COMPOSE_PROJECT_NAME:-devstack}.frontend-app-course-authoring" | |||
| environment: | |||
| PARAGON_BRAND_PACKAGE: '@edx/brand-edx.org' | |||
There was a problem hiding this comment.
It appears this MFE does not install @edx/brand-edx.org in the standard way, instead manually installing it in the gocd pipeline itself (source).
That said, it currently installs 2.x, not latest.
docker-compose.yml
Outdated
| @@ -767,6 +775,8 @@ services: | |||
| service: microfrontend | |||
| working_dir: '/edx/app/frontend-app-gradebook' | |||
| container_name: "edx.${COMPOSE_PROJECT_NAME:-devstack}.frontend-app-gradebook" | |||
| environment: | |||
| PARAGON_BRAND_PACKAGE: '@edx/brand-edx.org' | |||
There was a problem hiding this comment.
Note: it doesn't appear frontend-app-gradebook uses @edx/brand-edx.org today. See production app in screenshot below; the button styles are from the default Paragon styles, not @edx/brand-edx.org.
Would we want to update the devstack MFE to use @edx/brand-edx.org when stage/prod does not? For this PR, I might recommend not adding @edx/brand-edx.org for MFEs that seemingly don't use it. You might consider coordinating with the owning teams to start using the @edx/brand-edx.org theme in stage/prod/devstack.
docker-compose.yml
Outdated
| working_dir: '/edx/app/frontend-app-ora-grading@2.x' | ||
| container_name: "edx.${COMPOSE_PROJECT_NAME:-devstack}.frontend-app-ora-grading" | ||
| environment: | ||
| PARAGON_BRAND_PACKAGE: '@edx/brand-edx.org' |
There was a problem hiding this comment.
The version 2.x was incorrectly added to the working_dir; it should be on the PARAGON_BRAND_PACKAGE.
Related, edx-internal doesn't define an NPM_ALIAS for this MFE (source). However, it is running the @edx/brand-edx.org theme, as it's installed by default in the upstream MFE package.json (source), which is incorrect as @edx/brand-edx.org is 2U/edX.org specific and should not be in openedx repo.
Might be worth coordinating with the owning team to get @edx/brand-edx.org used within NPM_ALIAS in edx-internal instead, and keeping @edx/brand-openedx installed by default in the MFE, which is convention.
docker-compose.yml
Outdated
| @@ -842,6 +860,8 @@ services: | |||
| service: microfrontend | |||
| working_dir: '/edx/app/frontend-app-library-authoring' | |||
| container_name: "edx.${COMPOSE_PROJECT_NAME:-devstack}.frontend-app-library-authoring" | |||
| environment: | |||
| PARAGON_BRAND_PACKAGE: '@edx/brand-edx.org' | |||
There was a problem hiding this comment.
Here's another instance where @edx/brand-edx.org is actually installed via the GoCD pipeline file vs. the typical NPM_ALIASES config (source). That said, the version appears to be 2.x.
| if [ -n "$(printenv PARAGON_BRAND_PACKAGE)" ]; then | ||
| npx paragon install-theme "$(printenv PARAGON_BRAND_PACKAGE)" || exit 1 |
There was a problem hiding this comment.
Looks great; seems to work well locally.
|
@adamstankiewicz I have updated the PR with your suggested changes. Regarding frontend-app-gradebook and frontend-app-ora-grading, I have informed arbi-bom. |
Description:
Automate setting the edx.org theme as the default for both the edx-platform and MFEs during devstack provisioning.
Related Ticket:
edx/edx-arch-experiments#824