⚠️ No Changeset found

Latest commit: f50212a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

I wonder if there are any collisions between social icon names and Google Symbol names 🤔 you want to check that or should I have a look?

I wonder if there are any collisions between social icon names and Google Symbol names 🤔 you want to check that or should I have a look?

I have checked via the following simple script.
There are a little bit duplicated name.

const isCollision = () => {
	const iSocialIcons = socialIcons.map((icon) => icon.toLowerCase());
	return icons.flatMap((iconSet) => {
		return iconSet.icons.filter((icon) => iSocialIcons.includes(icon.toLowerCase()));
	});
};

Duplicated Names:
 ['android', 'bluetooth', 'usb', 'hive', 'php']

Thank you for confirming that!

Then I think a possible option is to prefix the social icon with the social- prefix and deal with automatically removing that prefix in the v-icon component, such that it is correctly rendered and current and future collisions can be avoided. Choosing the - as a separator also prevents us from mixing those up with the Google Material icons, as those are always snake_case.

So the logic update in v-icon to the socialIconName should be something like

• Does it start with social- -> Strip social-
• Otherwise the old logic applies

Thank you for confirming that!

Then I think a possible option is to prefix the social icon with the social- prefix and deal with automatically removing that prefix in the v-icon component, such that it is correctly rendered and current and future collisions can be avoided. Choosing the - as a separator also prevents us from mixing those up with the Google Material icons, as those are always snake_case.

So the logic update in v-icon to the socialIconName should be something like

• Does it start with social- -> Strip social-
• Otherwise the old logic applies

Thanks for the prefix suggestion!
I have implemented it.
It still works.

Looks like there is some weirdness going on with social icons in the v-icon component and the icon sizes, that is unrelated to this PR, but surfaces just now. @SP12893678 do you want to take a stab at that, otherwise I'll deal with it in a follow up PR.

Looks like there is some weirdness going on with social icons in the v-icon component and the icon sizes, that is unrelated to this PR, but surfaces just now. @SP12893678 do you want to take a stab at that, otherwise I'll deal with it in a follow up PR.

Sure! I found it should be set width: 100%, height: 100%, instead of auto
The following bluetooth case are Google Symbol scoped.
I have fixed the size problem both.

The following bluetooth case are Google Symbol scoped.

Ah.. damn, I think you found a case I did not think about initially. The prefix does not do anything for us, as the social icons always get precedence. So bluetooth will always end up being a social icon. :/

Any smart ideas here? Otherwise I'll give it a think and get back to you.

@hanneskuettner Lets follow the logic in v-icon for that. As far as I can tell, it'll prioritize the social icons over material icons

Maybe some user choosed the Google Symbol, but actually display social icon(fontawesome)
So user will expect display the social icon.
keep the logic may better？

So, to summarize, right now, for duplicated icons between social and google material symbols the user always saw the social icon rendered as those were preferred over the google material icons by default. I don't know if I like that implicit preference and personally would rather make it explicit.

We have to solutions:

1. Keep the current behavior, remove the social- prefix again and stick with what we have, the social icons taking preference in case of name collisions
2. Introduce a small breaking change that probably impacts very few users by requiring the social- prefix to the present for social icons and explicitly disambiguate the two sets. Internally we only use the social icons in two places, so updating that does not pose a problem for us.

@rijkvanzanten any preferences here?

Lets go with option number 2 👍🏻

Lets go with option number 2 👍🏻

I have enforced that social icons must include the social prefix in their usage