refactor: make images dynamically loaded#149
Conversation
jgmejilla
changed the title
There was a problem hiding this comment.
Thank you very much, and good work on this PR!
Admittedly, I'm very late with my review on this, and I would like to apologize for that. I may also need to commit the requested changes myself just so we can get this out in time.
I just have a couple of changes I'd like to request. In particular, I'm opting that we go back to using loading="lazy" since, in retrospect, I don't believe eager loading offers any advantage over lazy loading for our case. Additionally, make sure to pass the linting and formatting checks.
For future work, we should consider setting up a CDN to speed up image loading and handle the distribution of images.
PS: Please work on a branch of a fork rather than directly on a branch of this repository. This isn't strictly necessary, but this is just how we prefer contributions to be done on this repository.
Co-authored-by: Ehren Castillo <59799738+ehrelevant@users.noreply.github.com>
Co-authored-by: Ehren Castillo <59799738+ehrelevant@users.noreply.github.com>
Co-authored-by: Ehren Castillo <59799738+ehrelevant@users.noreply.github.com>
fix(EventCard): make image expand as expected Co-authored-by: Ehren Castillo <59799738+ehrelevant@users.noreply.github.com>
ehrelevant
left a comment
ehrelevant
left a comment
There was a problem hiding this comment.
Alright, LGTM!!!
We'll have to review this PR again in the future, but this is more than enough given our current time constraints. Thank you again, and congrats to both of you on your first PR to this repository!
|
Oh, and one additional thing to mention. Please read RFC-01 when you have the time. Particularly, section 4.2 on signing keys. Thanks! |
3 participants
Closes #146.
This PR changes how most images (specifically, images of members and events) are loaded. Rather than having the images be part of the build file, they are now fetched from an external server, and this drastically reduces the build size.