chore: Switch to using nuxt icon module #4493
Conversation
- setup module in tailwind-components lib - add molgenis icons to the setup - add samplepage to view/text icons in playground - 'fix' the svg's to use current fill
| @@ -69,7 +69,7 @@ const iconPositionClass = computed(() => { | |||
| class="flex items-center border rounded-full" | |||
| > | |||
| <span v-if="icon"> | |||
| <BaseIcon :name="icon" /> | |||
| <Icon :name="icon" /> | |||
There was a problem hiding this comment.
use the Icon component from the @nuxt/icon module instead of our own BaseIcon
good point, we are currently not really generating icons ( we generate vue components that are minimal wrappers around svg icons), and yes we could.
in non ssr mode we could make a smaller bundle, but this is by no means simple ( the setup is not that hard , but the code flow and (server/ops) config is pretty complex. All in all, now that we have it (sort of) working i'm not entirely convinced its worth the effort. on the plus side we remove a custom build job that relies on the sgvo dep, but now we have a dep on @nuxt/icon that is more powerfull but may also be more complex. the ability to add any icon of the 200,000+ icon at any time is cool, but not really something we need imo |
| - path: /api/_nuxt_icon/ | ||
| pathType: Prefix | ||
| backend: | ||
| service: | ||
| name: ssr-catalogue | ||
| port: | ||
| number: 3000 |
There was a problem hiding this comment.
this opens up the node '_nuxt_icon/ end-point' ( i.e. requests containing /api/_nuxt_icon/ get send to nuxt server ( on 3000) instead of the default port ( java on 8080)
apps/ui/app.config.ts
Outdated
There was a problem hiding this comment.
Maybe we could move the commonly used icons/aliases and settings into a shared configuration file? Maybe apps/icons.config.ts
There was a problem hiding this comment.
yea i initially had some issues with the layers and loading components with the same prefix from different layers , ill try to dedupe the config code , the duplicate code was annoying me as well
There was a problem hiding this comment.
i have tried serval options but have not been able to remove the per app config, it is needed as a work around for a limitation of the nuxt/icons , nuxt/layer combination. The additional per app icon prefix is needed to point the app to the correct relative virtual icon dir ( i.e. within the components this its a different location then from within a app using the components). the app.config.ts is a nuxt specific name , using further aliases to combine multiple app.config.ts files into a single file, would further complicate an already pretty complex files loading situation.
This limitations of the nuxt/icons, nuxt/layer combination ( no (smart) path override or combining icons from different locations into a single icons namespace) would be my main objection to using the nuxt/icons, if we do not want to accept the workaround a suggest we stick with out own custom icon loading solution
There was a problem hiding this comment.
(A reminder for later)
We may need to adjust the layout when there is only one page of a results. While testing this component in the catalogue app and the UI app, the next and back buttons do not appear (which is expected) but the background circle is still displayed. This should either be removed when there is only 1 page or the buttons should be disabled.
There was a problem hiding this comment.
not a 100% on what you mean here , but i think i get it , maybe a separate issue ? or is this related to this pr ?
apps/tailwind-components/README.md
Outdated
|
|
||
| This script uses the [sgvo](https://github.com/svg/svgo) module to clean the icons and transform them into vue components. These files are then saved in the `global/icons/` folder. | ||
| Icons are rendered using @nuxt/icon, for usage details and options see: https://nuxt.com/modules/icon | ||
| The custom (default) icons used in the component library are stored in the assets/icons folder |
There was a problem hiding this comment.
anything special one needs to do to these icons?
There was a problem hiding this comment.
the details are described in https://nuxt.com/modules/icon( i'll add a note in the use of the custom icons after addressing the other pr change requests.)
In practice, yes and ( and yes ) , in theorie no but we would need additional code ( like the code we are replacing ) to put the svg ( these are still the basis, used current and setup used in this pr ). Note: currently the tailwind-components module is served as nuxt app during dev and playground mode. The individual vue components do not require nuxt ( directly) but need nuxts autowire system to resolve the imports. |
|
There was a problem hiding this comment.
Wow! It surprises me that the dimensions are so large.
|
Close in favour of our own solution ( see Readme for spec on our solution). Main objection to this pr is that it would basically force all users of the component lib to use nuxt, and we want to keep the option open to write a plain vue app ( using the tailwind components) |
closes #4251
What are the main changes you did:
setup module in tailwind-components lib
add molgenis icons to the setup
add samplepage to view/text icons in playground
'fix' the svg's to use current fill
remove 'old' icons and tooling
rename custom icon files to follow nuxt/icon name schema ( kebab-case )
open up icon route via ingress ( for k8, vm's need update of nginx config⚠️ )
setup app config for playground, catalogue and ui apps
test local
test non ssr playground setup
test ssr app usage setup ( catalogue )