Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Performance] Photo optimization #444

Open
Tracked by #432
mwpark2014 opened this issue Feb 4, 2023 · 12 comments
Open
Tracked by #432

[Performance] Photo optimization #444

mwpark2014 opened this issue Feb 4, 2023 · 12 comments
Labels
epic Large body of work that can be broken down into a number of smaller issues frontend good first issue Good for newcomers help wanted Extra attention is needed

Comments

@mwpark2014
Copy link
Contributor

mwpark2014 commented Feb 4, 2023

Non-trivial project for optimizing photos. Need someone to investigate and drive

At least some of these should be considered doing within the server or CDN layer

If scope ends up being large, please break down into smaller tasks / git issues!

@mwpark2014 mwpark2014 changed the title Photo optimization [Performance] Photo optimization Feb 4, 2023
@zoobot zoobot added good first issue Good for newcomers help wanted Extra attention is needed frontend epic Large body of work that can be broken down into a number of smaller issues labels Feb 4, 2023
@ri0nardo
Copy link

@mwpark2014 is the link of the jpeg to the 404 error page?

@mwpark2014
Copy link
Contributor Author

I'm actually not sure, since that example is originally from @zoobot

@ri0nardo
Copy link

now looking at it, its from the 404 error page. I can mock up something for the 404 with an svg and text. That would eliminate the need to load a photo. The 404 should be the least intensive page that loads. Will create a separate issue for that.

But WebP is the move especially for the amount of images that will be present from the tree details, and in the future user uploads from selfies.

@zoobot
Copy link
Member

zoobot commented Feb 13, 2023

now looking at it, its from the 404 error page. I can mock up something for the 404 with an svg and text. That would eliminate the need to load a photo. The 404 should be the least intensive page that loads. Will create a separate issue for that.

Thanks, good idea to have no photo on 404.

@zoobot
Copy link
Member

zoobot commented Feb 13, 2023

I'm actually not sure, since that example is originally from @zoobot

@ri0nardo It is a link for the 404 I think... Someone added that on a redesign or on adding a 404 page for the first time.

@Tijani-zainab
What do you think about adding a note on the PR template to check image size before merging?

And specify we should be using webp or svg?

@mwpark2014
Copy link
Contributor Author

now looking at it, its from the 404 error page. I can mock up something for the 404 with an svg and text. That would eliminate the need to load a photo. The 404 should be the least intensive page that loads. Will create a separate issue for that.

But WebP is the move especially for the amount of images that will be present from the tree details, and in the future user uploads from selfies.

If it's on the 404 page, it's very unlikely that we need to eliminate the photo. The 404 page will be very lightweight since there's no other assets and certainly no JS loading
image

@mwpark2014
Copy link
Contributor Author

@Tijani-zainab What do you think about adding a note on the PR template to check image size before merging?

And specify we should be using webp or svg?

IMO, let's delegate someone to look into this task and not have other developers worry about it. Let's try to avoid premature optimization. Most photos aren't loaded on page load iirc, so I wouldn't be too sure that switching to webp is worthwhile. It's also relatively easy to go back at some point and swap out all .svg|png|jpeg|etc for webp in the future when the time comes

@zoobot
Copy link
Member

zoobot commented Feb 14, 2023

These pages, including the 404(NotFound) are wrapped in Suspense but not lazy loaded...
We should lazy load it.

import About from '@/pages/About/About';
import Privacy from '@/pages/Privacy/Privacy';
import License from '@/pages/License/License';
import UserProfile from '@/pages/Userprofile/UserProfile';
import Contact from '@/pages/Contact/Contact';
import NotFound from '@/pages/NotFound/NotFound';

@zoobot
Copy link
Member

zoobot commented Feb 14, 2023

#476
Just adding this 404 here because its awesome and relates to this thread.

@ri0nardo
Copy link

#476 Just adding this 404 here because its awesome and relates to this thread.

I want to save this for a new person that came last week that wanted to do frontend work. Perfect task for anyone new

@mwpark2014
Copy link
Contributor Author

#476 Just adding this 404 here because its awesome and relates to this thread.

I agree that this is very pleasing to the eye :D

@ri0nardo ri0nardo moved this from Todo to Help Wanted in Water the Trees Mar 6, 2023
@Tijani-zainab
Copy link
Contributor

I'm actually not sure, since that example is originally from @zoobot

@ri0nardo It is a link for the 404 I think... Someone added that on a redesign or on adding a 404 page for the first time.

@Tijani-zainab What do you think about adding a note on the PR template to check image size before merging?

And specify we should be using webp or svg?

Hello @zoobot Oh, I could do that.
will edit it and get a PR going for it. Thank you for pointing that out!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
epic Large body of work that can be broken down into a number of smaller issues frontend good first issue Good for newcomers help wanted Extra attention is needed
Projects
Status: Help Wanted
Development

No branches or pull requests

4 participants