search

overhangio / tutor-mfe

This plugin makes it possible to easily add micro frontend (MFE) applications on top of an Open edX platform that runs with Tutor.
GNU Affero General Public License v3.0
22 stars 95 forks source link

Logo missing on login page #99

Closed misilot closed 1 year ago

misilot commented 1 year ago

Bug description In Tutor v15 the Auth/Login MFE is not showing a logo How to reproduce Go to https://apps.demo.openedx.overhang.io/authn/login?next=%2F and see that the logo on the top left is not rendered. Only the text Open edx Demo Site is showing up.

Environment

Additional context

insad commented 1 year ago

And the login page is not styled now at all (I'm using an adapted indigo template). Frankly horrible looking login page 😥

regisb commented 1 year ago

Thanks for the report @misilot. I'm moving this issue to the tutor-mfe repository, which runs the authn MFE. Would you like to open a PR to fix this issue?

arbrandes commented 1 year ago

Matters of taste aside, the authn MFE is definitely stylable. As noted in the forum, it is possible to change the logo and colors.

If you'd like the MFE to be more stylable, or for it to have a better default look, please open an issue (or preferrably, a PR) in the https://github.com/openedx/frontend-app-authn repository.

If you'd like tutor-indigo to support it, please open an issue or PR in the https://github.com/overhangio/tutor-indigo repository.

Thank you for the report!

misilot commented 1 year ago

@arbrandes shouldn't there be some logo? Instead of a missing image by default? Just curious

image
arbrandes commented 1 year ago

@misilot, thanks for the screenshot, it makes the issue much clearer. I did not notice before we were only showing the alt text with a broken link (because Firefox doesn't show a broken image, just renders the alt text). This does indeed warrant discussion. Reopening.

As for the fix, the easy way out here is to set LOGO_URL by default to https://edx-cdn.org/v3/default/logo.svg, which is the default for development purposes. But:

  1. That throws in a non-Open edX org, non-Overhang org external dependency we don't control. The logo could go away any time, and get us back into this situation.
  2. Plus, any external logo could be (and probably is) tracked.
  3. Anybody deploying this "for real" will want to change that URL anyway.
  4. We could use one of the logos in https://logos.openedx.org/, but currently none of those would fit very well, I think. We could add a more desirable logo. But then we'd still want to warn people that their installations could be tracked.
  5. We could open an issue/PR upstream to have the logo default to something served by the MFE itself.
  6. We could open an issue/PR upstream to have frontend-app-authn serve a logo by the openedx-brand package, like frontend-component-header ~does~ should.

I think option 6 is The Right Way ™️. @regisb, @kdmccormick, thoughts?

arbrandes commented 1 year ago

Oh, wait, we have static/images/logo.png in the LMS! And we set LOGO_URL to it by default! Huh. Might still be a bug upstream in the authn repo, but let me investigate a little.