Updating style assets #110
Conversation
There was a problem hiding this comment.
Looks fantastic, and code looks well organised. Sorry I don't know more about CSS to give more constructive feedback.
If I had to nitpick I'd say the transition where the side menu pops out is a little bit quick, and maybe the colour change on hover for the site title is unnecessary? But also feel free to ignore me because clearly you know what you're doing.
Migrations for 'mod_app':
mod_app/migrations/0023_alter_bibliographyitem_full_citation_and_more.py
- Alter field full_citation on bibliographyitem
- Alter field short_citation on bibliographyitem
- Alter field format_type on film
- Alter field bibliography on projectnote
- Create model Feedback
| @@ -22,6 +22,7 @@ class VideoInline(PreviewMixin, s3BrowserButtonMixin, admin.TabularInline): | |||
| "grp-collapse", | |||
| "grp-closed", | |||
| ] | |||
| exclude = ("file",) | |||
There was a problem hiding this comment.
is this now excluded because files are no longer used for videos (only links)?
There was a problem hiding this comment.
yep! All the other models that are based on links can still use files, and technically speaking, you could add a file to a video instance still, but you'd have to do it through the command line. This just disables it from being shown in the admin frontend so the researchers can't use it
| margin-bottom: 1rem; | ||
| display: flex; | ||
| columns: 2; | ||
| margin: 1vw 6vw; |
There was a problem hiding this comment.
just curious why the change from rem to vw?
There was a problem hiding this comment.
It might just be personal bias but I felt like it worked a bit better for responsiveness - maybe since it's based off the width of the screen instead of an element?
| '<img src="{}" style="max-width: 15rem;" />', | ||
| obj.url, | ||
| ) | ||
| if obj.__class__.__name__ == "Video": |
There was a problem hiding this comment.
| if obj.__class__.__name__ == "Video": | |
| if isinstance(obj, Video): |
would it make sense to do this instead (and add from mod_app.models.support_models import Video to the imports? Or perhaps obj._meta.verbose_name == "Video"?
There was a problem hiding this comment.
I'll try it! I know I ran into circular import issues before but it might not be the case with this file
There was a problem hiding this comment.
The isinstance didn't seem to work when I tested it, is there a particular benefit to using obj._meta.verbose_name == "Video" instead of the obj.__class__.__name__?
|
Aylin would like us to use Montserrat Bold for the header instead of Impact and remove the film strip styling for the footer. Content should be dark-mode forward with queries for light mode preference, also a toggle option to view the site in light or dark mode |
Have improved our basic components in alignment with the new mockups and have added some basis for mobile viewing:
film list view on desktop - new cards that are common across the main models (image at top of film cards will be better integrated in future as background [tbc]), more consistent buttons, new header layout + background (word logo to be updated in future)
same view on mobile:
footer on desktop: (streamlined code and more responsive + collaborator logos)
on mobile: (decision to not include own logo based on looking at other companies' mobile footers)
closes #106 , #99 , #105