infi-nl / the-infi-way

How we like to build software
https://way.infi.nl
Other
9 stars 8 forks source link

Icons in list #76

Closed yougotdirked closed 1 year ago

yougotdirked commented 1 year ago

-added icons to the list with items -icons are stored in .svg files -added necessary css classes

netlify[bot] commented 1 year ago

Deploy Preview for the-infi-way ready!

Name Link
Latest commit 736af576252981d5d0af169b20e523724b027d6b
Latest deploy log https://app.netlify.com/sites/the-infi-way/deploys/64d4a4f420d81100080afd2c
Deploy Preview https://deploy-preview-76--the-infi-way.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.

LucaScorpion commented 1 year ago

With these assets now in separate files, does it make sense to also replace the inline <svg> elements?

yougotdirked commented 1 year ago

I think its wise to leave the svg tags inline as much as possible as it gives way more control. I used tags because there was no way around this, unfortunately. A case can be made that it should be unified, though - but the image tag is the "lowest common denominator" i guess

LucaScorpion commented 1 year ago

@yougotdirked I see. The consistency was indeed my main point/worry :sweat_smile: Since with this setup we define the SVGs in 2 places (inline and in a file). I tried playing around with it locally, and there it does seem to work fine if we use an img tag instead of the inline svg. The .block-image svg { CSS selector then needs to be changed to .block-image img {, but that seems to be all that's needed. So while the inline SVGs do give more control, it doesn't seem like we need that functionality. This approach would have a (slight) preference from me.

Edit: if we do use the external files, I also see a possibility to greatly simplify the template, since that would potentially mean we could just loop over all topics instead of having to define them all in the HTML...