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

Allow Vuetify custom icons #409

Merged
merged 3 commits into from
Jul 23, 2024

Conversation

sronveaux
Copy link
Collaborator

@sronveaux sronveaux commented Jul 12, 2024

This PR opens the possibility to define custom icons to be injected inside Vuetify.

screenshot

Just to give a little bit more context and explain some design choices around this:

Three methods were tested to define and import those custom icons:

  1. Defining an SVG path exported as a string from a .js file. This method which is the one implemented in this PR is the simplest one which mimicks how mdi itself is written (see).
  2. Importing SVG files directly using an appropriate loader. This methods necessitates to add a custom loader for Webpack and tweak the configuration. The standard loaders available out there are often not up-to-date and don't import icons in a format which can be resized or colorized properly as explained in Vuetify documentation. A way to circumvent this would be to write our own Webpack loader as a separate package to publish and maintain on NPM which is a bit overkill from my point of view.
  3. Importing VUE components which embeds an SVG directly (see the same page as for point 2) which works but necessitates to rewrite a component for each and every icon and to add styling inside for it to embed properly.

A first implementation was made that imported those three formats, which explains the way the IconUtil file is written. This structure was kept in case such a behaviour was needed in the future.

However, only the first method was kept in the end as this is the only one that can be migrated to Vuetify3 easily. Components would need to be written very differently to be used correctly by Vuetify3 which makes the second and third method useless for now as someone writing icons in such formats would need to rewrite them completely when migrating later...

If this PR is eventually accepted and merged, just let me know if you want me to add this to the workshop. We could for example demonstrate how to change the icon of the custom component. This could be great and funny if a decision is taken about #116 one day !

Copy link
Collaborator

@chrismayer chrismayer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this nice feature @sronveaux 👍 I like the possibility to add custom SVG icons and the simplicity. I second your choice not to go for a custom Webpack loader et.al. The approach is easily portable to future developments.

Code looks good and I tested it successfully.

One little remark, since I mixed it up while testing: There is already a folder icon and I placed my custom SVG-icon in there and it surely did not show in the app since it is the wrong folder. Your extension looks in folder icons. Maybe another name could reduce the chance to mess those folders up, such as custom-icons or similar? Or is it just my dumbness? I leave it up to you, whether to change or not. I am fine with both.

I also like the idea to have this in feature in the workshop. A perfect spot to illustrate the possibilities.

src/util/Icon.js Show resolved Hide resolved
@sronveaux
Copy link
Collaborator Author

Hi @chrismayer and thanks for the review !

I'll address the comment you made shortly...

Just coming back to you to have your opinion about the icons folder. Sure I can rename it without problem. As stated in the docs, it is not in app/static where you already find the icon folder but directly in app/icons, as placing them inside static would also copy them in the dist folder while building. As they need to be bundled to get caught by Vuetify, I got them out of static.

I also thought about creating an empty icons directory inside the app-starter so people would directly see where to put them. That's also why I had to "overcomplicate" the regex looking for icons as the icons directory doesn't necessarily exist... Just wanted to know if you would find it clearer if there was an empty custom-icons created inside the app-stater ?

And thinking about the future, do you have an idea for the icon to assign in the workshop ? The W I've put in the docs looks a little too simple I think... Perhaps I can reuse the idea of @justb4 or do you have something else in mind ?

@sronveaux
Copy link
Collaborator Author

Hi @chrismayer,

I should have addressed all you have suggested here... could you have another look please ?

@chrismayer
Copy link
Collaborator

Very nice, thanks for your ongoing effort here @sronveaux! Looks good from my side. Please merge at will 👍

@chrismayer
Copy link
Collaborator

And thinking about the future, do you have an idea for the icon to assign in the workshop ? The W I've put in the docs looks a little too simple I think... Perhaps I can reuse the idea of @justb4 or do you have something else in mind ?

With some temporal distance I have to admit that I like the logo suggestion you linked. If you are familiar with graphics or you want to present/discuss something let's move the discussion in the existing issue #116 Would be really cool to have a logo after all the time...

@sronveaux sronveaux merged commit 3a33aae into wegue-oss:master Jul 23, 2024
1 check passed
@sronveaux sronveaux deleted the vuetify-custom-icons branch July 23, 2024 07:29
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