komiser
komiser copied to clipboard
Tech improvement: Refactor direct embedding of SVGs
Is your feature request related to a problem? Please describe. Could be better if we can refactor the frontend components/pages to use SVGs from the public folder rather than hardcoding the SVGs as inline embeddings
Describe the solution you'd like
Use next/image
to serve the SVGs instead of using inlines. We could even use a library like svgr
https://github.com/gregberge/svgr to do this. Would like to analyze.
Additional context Reference: https://github.com/tailwarden/komiser/blob/33567623e46ce27c7f8c9230c6d144139491466c/dashboard/components/dashboard/components/top-stats/DashboardTopStatsCards.tsx#L21
Am i willing to submit a PR ? YES
@ShubhamPalriwala @Traxmaxx wdyt?
Is your feature request related to a problem? Please describe. Could be better if we can refactor the frontend components/pages to use SVGs from the public folder rather than hardcoding the SVGs as inline embeddings
Describe the solution you'd like Use
next/image
to serve the SVGs instead of using inlines. We could even use a library likesvgr
https://github.com/gregberge/svgr to do this. Would like to analyze.Additional context Reference:
https://github.com/tailwarden/komiser/blob/33567623e46ce27c7f8c9230c6d144139491466c/dashboard/components/dashboard/components/top-stats/DashboardTopStatsCards.tsx#L21
Am i willing to submit a PR ? YES
Hey transferring SVGs to the public folder and using Next/image to serve them might not be necessary because Next/image does not optimize SVGs. You can find more information about this in the Next.js documentation here: https://nextjs.org/docs/pages/api-reference/components/image#dangerouslyallowsvg
the current way the SVGs are rendered are also not optimised, the next js documentation means compression as optimization.
I just added #1080 shall I merge these two issues and update the desciption? It's good to get startet on it.
edit: I have no particular opinion if we want to load them via public paths or as react components as long as icons do not need to be animated at some point. In this case it's better to have them as react components. It also makes overriding styles with tailwind easier maybe 🤔
Description from the other issue just for reference:
Is your feature request related to a problem? Please describe. We want to have an overview of existing Icons and which ones to add when working on new screens.
Describe the solution you'd like We should add all icons into a shared folder inder
dashboard/components/icons
and add all of them to storybook to make them searchable. Then replace the static ones with the new components.Currently an static icon is inline svg. For example:
<svg width="24" height="24" viewBox="0 0 24 24" fill="none" xmlns="http://www.w3.org/2000/svg" > <path d="M19.7897 14.9298C17.7297 16.9798 14.7797 17.6098 12.1897 16.7998L7.47966 21.4998C7.13966 21.8498 6.46966 22.0598 5.98966 21.9898L3.80966 21.6898C3.08966 21.5898 2.41966 20.9098 2.30966 20.1898L2.00966 18.0098C1.93966 17.5298 2.16966 16.8598 2.49966 16.5198L7.19966 11.8198C6.39966 9.21982 7.01966 6.26982 9.07966 4.21982C12.0297 1.26982 16.8197 1.26982 19.7797 4.21982C22.7397 7.16982 22.7397 11.9798 19.7897 14.9298Z" stroke="#697372" stroke-width="1.5" stroke-miterlimit="10" stroke-linecap="round" stroke-linejoin="round" /> <path d="M6.88965 17.4902L9.18965 19.7902" stroke="#697372" stroke-width="1.5" stroke-miterlimit="10" stroke-linecap="round" stroke-linejoin="round" /> <path d="M14.5 11C15.3284 11 16 10.3284 16 9.5C16 8.67157 15.3284 8 14.5 8C13.6716 8 13 8.67157 13 9.5C13 10.3284 13.6716 11 14.5 11Z" stroke="#697372" stroke-width="1.5" stroke-linecap="round" stroke-linejoin="round" /> </svg>
These static icons need to become a component. For example
import { SVGProps } from 'react'; const KeyIcon = (props: SVGProps<SVGSVGElement>) => ( <svg width="24" height="24" viewBox="0 0 24 24" fill="none" xmlns="http://www.w3.org/2000/svg" {...props} > <path d="M19.7897 14.9298C17.7297 16.9798 14.7797 17.6098 12.1897 16.7998L7.47966 21.4998C7.13966 21.8498 6.46966 22.0598 5.98966 21.9898L3.80966 21.6898C3.08966 21.5898 2.41966 20.9098 2.30966 20.1898L2.00966 18.0098C1.93966 17.5298 2.16966 16.8598 2.49966 16.5198L7.19966 11.8198C6.39966 9.21982 7.01966 6.26982 9.07966 4.21982C12.0297 1.26982 16.8197 1.26982 19.7797 4.21982C22.7397 7.16982 22.7397 11.9798 19.7897 14.9298Z" stroke="#697372" stroke-width="1.5" stroke-miterlimit="10" stroke-linecap="round" stroke-linejoin="round" /> <path d="M6.88965 17.4902L9.18965 19.7902" stroke="#697372" stroke-width="1.5" stroke-miterlimit="10" stroke-linecap="round" stroke-linejoin="round" /> <path d="M14.5 11C15.3284 11 16 10.3284 16 9.5C16 8.67157 15.3284 8 14.5 8C13.6716 8 13 8.67157 13 9.5C13 10.3284 13.6716 11 14.5 11Z" stroke="#697372" stroke-width="1.5" stroke-linecap="round" stroke-linejoin="round" /> </svg> ); export default KeyIcon;
Beware of classes or stylings which need to be passed in as props when replacing the static icon!
Describe alternatives you've considered None
please assign me
Sorry @rafiya2003 I just saw this now. All yours