EssamWisam / cmp-docs

A comprehensive guide for prospective, current and past students in the computer engineering department of Cairo university.
https://cmp-docs.pages.dev
52 stars 8 forks source link

🐛 Fix current markdown detection bug #41

Closed EssamWisam closed 5 months ago

EssamWisam commented 5 months ago

I remember that @Iten-No-404 had a mechanism in place that would detect if loading LinkedIn image has failed and render the thumb if it has. It turns out that reason it didn't work (hence, squished images) was that the currentMarkdown when in the courses page was ./department/... while ../department was what's in the if check. I don't know when did that change but I believe it worked correctly when it was first implemented.

This is meant to be a temporary measure until @KnockerPulsar finds time to integrate the LinkedIn scraping feature he worked on. #29 #33

netlify[bot] commented 5 months ago

Deploy Preview for cmp-docs ready!

Name Link
Latest commit 1097d15928bc75745fd766b213f7f64c7623908d
Latest deploy log https://app.netlify.com/sites/cmp-docs/deploys/65bb75428d05da0008091ba6
Deploy Preview https://deploy-preview-41--cmp-docs.netlify.app/
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

vercel[bot] commented 5 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
cmp-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 1, 2024 10:41am
EssamWisam commented 5 months ago

Hmmm. perfectly fixes it offline but squished images still show up on initial load online for some reason. Will try to evade making the LinkedIn request in the first place to see if this solves it.

EssamWisam commented 5 months ago

Didn't fix it so I reverted. Not the biggest deal ever since it only happens in the initial load of the website (first itme the browser sees it). Further ones load the images with no problem. Also think that if the internet is fast enough so that the images are loaded from the server before the components do in the initial load then it solves it.

EssamWisam commented 5 months ago

Made one more commit to force the image to take height in all cases so that even if it fails to load in the initial loading it doesn't squish and stays circular.

EssamWisam commented 5 months ago

@Iten-No-404 merge at your leisure should this PR make sense.

Iten-No-404 commented 5 months ago

@Iten-No-404 merge at your leisure should this PR make sense.

@EssamWisam, The fix seems to be working properly. In the Netlify deployment, the initial load is always successful. I tried it 10 times with different circumstances (for example: reloading, launching the website on the classes pages directly, & launching the website on another page then navigating to it). As for the Vercel one, I think I don't have access to the deployment preview.

EssamWisam commented 5 months ago

Could also be that internet is faster at your side. The check I do is launch the website from an incognito tab so force the browser not to use any cache.

As for the Vercel one, I think I don't have access to the deployment preview.

I find it comical that they want users to pay to be able to invite... I can assure you that for me the behaviour was consistent across both.

Iten-No-404 commented 5 months ago

Could also be that internet is faster at your side.

I doubt it but maybe.

The check I do is launch the website from an incognito tab so force the browser not to use any cache.

I sometimes launched it in incognito mode and when just reloading the page, I do a deep reload.

I find it comical that they want users to pay to be able to invite...

Many companies monetize the most basic features these days...🤦🏻‍♀️

I can assure you that for me the behaviour was consistent across both.

Alright, how about trying again in a few hours? The internet speed is bound to be different then. Also, I tried a couple of times more and I noticed something. Sometimes, about only the first half of the list loads perfectly while the second half barely loads.

image

I can merge this PR and try it out a bit more on the Vercel deployment instead. Either way, it's much better than the squished images. Nice work.

EssamWisam commented 5 months ago

Also, I tried a couple of times more and I noticed something. Sometimes, about only the first half of the list loads perfectly while the second half barely loads.

Yes, the ones that barely load correspond to squished images (now white disks). This should no longer happen upon refresh or visiting site again from same browser or at least this is what happens to me.

Iten-No-404 commented 5 months ago

Tried a couple of times in both deployments and the squished images bug doesn't happen at all which is good news. I have noticed that Vercel is a bit slower at loading the images than Netlify. Nonetheless, the bug doesn't occur and at worst some of the images (in the second half of the list) aren't loaded and appear as white circles instead which is still better than the bug.

This should no longer happen upon refresh or visiting site again from same browser or at least this is what happens to me.

Indeed, that's what I have experienced as well.

EssamWisam commented 5 months ago

Iten, let me elaborate, I do expect the squished images bug not to occur because I modified the code so that instead of the image being "squished", it becomes a white disk as in my comments above. So this converts the squished images bug into empty images bug.

What I was saying earlier is that the empty images bug happens for me only in the initial load from an icognito tab/new browser. It also only happens starting from about half the list like you showed.

Indeed, that's what I have experienced as well.

Okay. Makes sense now.