search

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

🐜🔧 Image Bug Fix #28

Closed Iten-No-404 closed 7 months ago

Iten-No-404 commented 7 months ago

Closes #27

Very naïve solution to the problem at hand where only people's images are created using the ImageComponent, the rest of the images are generated using the img tag.

netlify[bot] commented 7 months ago

Deploy Preview for cmp-docs ready!

Name Link
Latest commit 414c3a8153f531bc752e80daa3d28a518fdb6ad3
Latest deploy log https://app.netlify.com/sites/cmp-docs/deploys/65565db54db8cc0008fee73f
Deploy Preview https://deploy-preview-28--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.

EssamWisam commented 7 months ago

This works and you could merge it if you like. An alternative approach that refrains from modifying the public files is to use the currentMarkdown state to infer whether the markdown (or yaml) rendered comes from the Classes folder. As a maintainer, you get to choose whether you prefer that or not and I will perfectly respect it.

Iten-No-404 commented 7 months ago

This works and you could merge it if you like. An alternative approach that refrains from modifying the public files is to use the currentMarkdown state to infer whether the markdown (or yaml) rendered comes from the Classes folder. As a maintainer, you get to choose whether you prefer that or not and I will perfectly respect it.

That would be a smarter option indeed. I have reverted the changes. Thank you for the suggestion. image

Also, I'd like to add that I sometimes find unloaded images as in the image above, especially in the other classes' pages. Checked out previous PRs and it seems like that the problem was there from the beginning, we just didn't notice it. So, I would assume that the invalid images are from the website that creates these placeholder images. Not sure if we should check for that as well or just opt for reloading the page (and so getting another seed which will highly be a valid image).

Either way, I think I will merge it for now and if we see this small issue increasing in frequency, we can either check for the validity of the placeholder image or use another placeholder website.