Fix/logos #1097
Fix/logos #1097
Conversation
| @media (max-width: 500px) { | ||
| margin-left: 10px; | ||
| height: $logo-height-mobile; | ||
| } |
There was a problem hiding this comment.
I would develop mobile-first and then use min-width to make exceptions for larger devices instead of vice versa.
| color: transparent; | ||
| width: 30%; | ||
|
|
||
| @media (max-width: 500px) { |
There was a problem hiding this comment.
I see you are using a breakpoint of 500px here. I just checked and found that MUSCLE's codebase now has breakpoints of 320, 350, 420, 500, 520, and 720 px.
First of all, I think that's an excessive amount. Maybe we can simplify that in the future and have only 2-3, maybe 4 breakpoints max.
Second, we already have breakpoints defined in variables.scss. Why not use those or add the ones you are using here?
MUSCLE/frontend/src/scss/variables.scss
Lines 28 to 33 in 38669ad
There was a problem hiding this comment.
I totally agree that we should clean this up. I copied and pasted this breakpoint from former code. I'll see about reusing the breakpoint from variables here instead.
| .aha__dashboard { | ||
|
|
||
| .aha__logo { | ||
| margin-left: 35%; |
There was a problem hiding this comment.
What's the reason for using a percentage as margin? Should it scale with device width?
| 'logoUrl': f'{settings.BASE_URL.strip("/")}/{settings.MEDIA_URL.strip("/")}/{str(theme.logo_image.file)}' if theme.logo_image else None, | ||
| 'backgroundUrl': f'{settings.BASE_URL.strip("/")}/{settings.MEDIA_URL.strip("/")}/{str(theme.background_image.file)}' if theme.background_image else None, |
There was a problem hiding this comment.
Can't you use the serialize_image method like in the other PRs of today?
MUSCLE/backend/image/serializers.py
Lines 8 to 13 in 38669ad
There was a problem hiding this comment.
Eventually, that's 100% the better way, yes. For now, the frontend changes involved to adapt to the objects here are kind of daunting (especially to use the href defined in the images).
This branch ensures that logos will be displayed correctly, i.e., centered and big on the dashboard, and smaller top-left for the experiments (these would require defining separate themes for now).