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

🐛 Fix current markdown detection bug #41

Merged
merged 4 commits into from
Feb 1, 2024
Merged

Conversation

EssamWisam
Copy link
Owner

@EssamWisam EssamWisam commented Feb 1, 2024

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

Copy link

netlify bot commented Feb 1, 2024

Deploy Preview for cmp-docs ready!

Name Link
🔨 Latest commit 1097d15
🔍 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.

Copy link

vercel bot commented Feb 1, 2024

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
Copy link
Owner Author

EssamWisam commented Feb 1, 2024

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
Copy link
Owner Author

EssamWisam commented Feb 1, 2024

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
Copy link
Owner Author

EssamWisam commented Feb 1, 2024

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
Copy link
Owner Author

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

@Iten-No-404
Copy link
Collaborator

@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
Copy link
Owner Author

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
Copy link
Collaborator

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.

@Iten-No-404 Iten-No-404 merged commit 0b9ef53 into main Feb 1, 2024
6 checks passed
@EssamWisam
Copy link
Owner Author

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
Copy link
Collaborator

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
Copy link
Owner Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants