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.

Sign up for GitHub

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

refactor: Custom icons #864

Merged
merged 13 commits into from
Feb 7, 2024
Merged

refactor: Custom icons #864

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
692bc99
fix: Custom icons
samuveth Feb 3, 2024
a83fd89
Merge branch 'master' into samuv/fix-icons
samuveth Feb 5, 2024
4999afe
fix: Use link color for more visibility on light mode
samuveth Feb 5, 2024
43850b2
Update src/views/Home.vue
samuveth Feb 5, 2024
af5b767
Update src/views/Home.vue
samuveth Feb 5, 2024
74e26f8
fix: Revert changes
samuveth Feb 5, 2024
6049cbd
Merge branches 'samuv/fix-icons' and 'samuv/fix-icons' of https://git…
samuveth Feb 5, 2024
9d48f50
Merge branch 'master' into samuv/fix-icons
samuveth Feb 6, 2024
d76b17a
fix: Size consistency using em
samuveth Feb 6, 2024
db9e707
fix: Use link color
samuveth Feb 6, 2024
a633726
fix: Use larger em value
samuveth Feb 6, 2024
49059f2
Merge branch 'master' into samuv/fix-icons
samuveth Feb 6, 2024
0a84135
fix: Use hard coded 20px again
samuveth Feb 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions src/assets/discord.svg
View file
Open in desktop

This file was deleted.

3 changes: 0 additions & 3 deletions src/assets/github.svg
View file
Open in desktop

This file was deleted.

4 changes: 4 additions & 0 deletions src/assets/icons/discord.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 4 additions & 0 deletions src/assets/icons/github.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
5 changes: 5 additions & 0 deletions src/assets/icons/x.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 0 additions & 3 deletions src/assets/x.svg
View file
Open in desktop

This file was deleted.

30 changes: 21 additions & 9 deletions src/views/Home.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,25 @@
<script setup lang="ts">
import { version, repository } from '@/../package.json';
import ICX from '~icons/c/x';
import ICDiscord from '~icons/c/discord';
import ICGithub from '~icons/c/github';

const COMMIT_SHA = import.meta.env.VITE_COMMIT_SHA || '';

const socials = [
samuveth marked this conversation as resolved.
Show resolved Hide resolved
{
href: 'https://twitter.com/SnapshotLabs',
icon: ICX
},
{
href: 'https://discord.gg/snapshot',
icon: ICDiscord
},
{
href: 'https://github.com/snapshot-labs',
icon: ICGithub
}
];
</script>

<template>
Expand Down Expand Up @@ -54,15 +72,9 @@ const COMMIT_SHA = import.meta.env.VITE_COMMIT_SHA || '';
</div>
<div class="space-y-2">
<div class="eyebrow">Join the community</div>
<div class="space-x-2">
<a href="https://twitter.com/SnapshotLabs" target="_blank">
<img src="~@/assets/x.svg" class="w-[32px] h-[32px] inline-block" />
</a>
<a href="https://discord.gg/snapshot" target="_blank">
<img src="~@/assets/discord.svg" class="w-[32px] h-[32px] inline-block" />
</a>
<a href="https://github.com/snapshot-labs" target="_blank">
<img src="~@/assets/github.svg" class="w-[32px] h-[32px] inline-block" />
<div class="flex space-x-2">
<a v-for="social in socials" :key="social.href" :href="social.href" target="_blank">
samuveth marked this conversation as resolved.
Show resolved Hide resolved
<component :is="social.icon" class="!text-[27px] text-[#606060] hover:text-skin-link" />
</a>
</div>
</div>
Expand Down
37 changes: 23 additions & 14 deletions src/views/Space/Overview.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@
import { _n, compareAddresses, sanitizeUrl } from '@/helpers/utils';
import { getNetwork, offchainNetworks } from '@/networks';
import { Space } from '@/types';
import ICX from '~icons/c/x';
import ICDiscord from '~icons/c/discord';
import ICGithub from '~icons/c/github';
import IHGlobeAlt from '~icons/heroicons-outline/globe-alt';

const PROPOSALS_LIMIT = 4;

Expand All @@ -24,7 +28,9 @@ const network = computed(() => getNetwork(props.space.network));
const spaceStarred = computed(() => spacesStore.starredSpacesIds.includes(spaceIdComposite));
const isController = computed(() => compareAddresses(props.space.controller, web3.value.account));

const externalUrl = computed(() => sanitizeUrl(props.space.external_url));
const externalUrl = computed(() =>
samuveth marked this conversation as resolved.
Show resolved Hide resolved
props.space.external_url ? sanitizeUrl(props.space.external_url) : null
);
const twitterUrl = computed(() =>
props.space.twitter ? sanitizeUrl(`https://twitter.com/${props.space.twitter}`) : null
);
Expand All @@ -35,6 +41,13 @@ const githubUrl = computed(() =>
props.space.github ? sanitizeUrl(`https://github.com/${props.space.github}`) : null
);

const socials = computed(() => [
{ href: externalUrl.value, icon: IHGlobeAlt },
{ href: twitterUrl.value, icon: ICX },
{ href: discordUrl.value, icon: ICDiscord },
{ href: githubUrl.value, icon: ICGithub }
]);

const proposalsRecord = computed(() => proposalsStore.proposals[spaceIdComposite]);

watchEffect(() => setTitle(props.space.name));
Expand Down Expand Up @@ -92,19 +105,15 @@ watchEffect(() => setTitle(props.space.name));
{{ space.about }}
</span>
</div>
<div class="space-x-2">
<a v-if="externalUrl" :href="externalUrl" target="_blank">
<IH-globe-alt class="w-[26px] h-[26px] inline-block text-[#606060]" />
</a>
<a v-if="twitterUrl" :href="twitterUrl" target="_blank">
<img src="~@/assets/x.svg" class="w-[26px] h-[26px] inline-block" />
</a>
<a v-if="discordUrl" :href="discordUrl" target="_blank">
<img src="~@/assets/discord.svg" class="w-[26px] h-[26px] inline-block" />
</a>
<a v-if="githubUrl" :href="githubUrl" target="_blank">
<img src="~@/assets/github.svg" class="w-[26px] h-[26px] inline-block" />
</a>
<div class="space-x-2 flex">
<span v-for="(social, i) in socials" :key="i">
<a v-if="social.href" :href="social.href" target="_blank">
<component
:is="social.icon"
class="!text-[22px] text-[#606060] hover:text-skin-text"
/>
</a>
</span>
</div>
</div>
</div>
Expand Down
10 changes: 8 additions & 2 deletions vite.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import AutoImport from 'unplugin-auto-import/vite';
import Components from 'unplugin-vue-components/vite';
import Icons from 'unplugin-icons/vite';
import IconsResolver from 'unplugin-icons/resolver';
import { FileSystemIconLoader } from 'unplugin-icons/loaders';
import visualizer from 'rollup-plugin-visualizer';
import inject from '@rollup/plugin-inject';

Expand All @@ -30,6 +31,7 @@ export default defineConfig({
directoryAsNamespace: true,
resolvers: [
IconsResolver({
customCollections: ['c'],
alias: {
h: 'heroicons-outline',
s: 'heroicons-solid'
Expand All @@ -45,8 +47,12 @@ export default defineConfig({
Icons({
compiler: 'vue3',
iconCustomizer(collection, icon, props) {
props.width = '20px';
props.height = '20px';
props.class = 'text-sm';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I switched the props to text-sm instead of using a 20-pixel width and height. This change simplifies setting the icon size. However, you need to prefix ! for overrides, like !text-lg.

Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer we not make icons some type of global size. I'd rather overwrite width/height to be 1em and then it will just inherit text-size from its parent (basically the same as we do with currentColor).

Then when we actually use it we can use regular font-size to control it (no need to use !important). It also makes it use size you actually request (because right now to achieve 32px you need to use text-[27px] and you actually end up with 27*1.2 so 32.4). Also it feels a bit more cleaner to set text size/color properties on a instead of this component itself to me (see the diff), but it's just nitpick.

diff --git a/src/views/Home.vue b/src/views/Home.vue
index 6ba834d..0b1d1d4 100644
--- a/src/views/Home.vue
+++ b/src/views/Home.vue
@@ -73,8 +73,14 @@ const socials = [
       <div class="space-y-2">
         <div class="eyebrow">Join the community</div>
         <div class="flex space-x-2">
-          <a v-for="social in socials" :key="social.href" :href="social.href" target="_blank">
-            <component :is="social.icon" class="!text-[27px] text-[#606060] hover:text-skin-link" />
+          <a
+            v-for="social in socials"
+            :key="social.href"
+            :href="social.href"
+            target="_blank"
+            class="text-[32px] text-[#606060] hover:text-skin-link"
+          >
+            <component :is="social.icon" />
           </a>
         </div>
       </div>
diff --git a/src/views/Space/Overview.vue b/src/views/Space/Overview.vue
index 528a786..ce8a165 100644
--- a/src/views/Space/Overview.vue
+++ b/src/views/Space/Overview.vue
@@ -107,11 +107,13 @@ watchEffect(() => setTitle(props.space.name));
         </div>
         <div class="space-x-2 flex">
           <span v-for="(social, i) in socials" :key="i">
-            <a v-if="social.href" :href="social.href" target="_blank">
-              <component
-                :is="social.icon"
-                class="!text-[22px] text-[#606060] hover:text-skin-text"
-              />
+            <a
+              v-if="social.href"
+              :href="social.href"
+              target="_blank"
+              class="text-[26px] text-[#606060] hover:text-skin-text"
+            >
+              <component :is="social.icon" />
             </a>
           </span>
         </div>
diff --git a/vite.config.ts b/vite.config.ts
index 86a6891..4fb81a0 100644
--- a/vite.config.ts
+++ b/vite.config.ts
@@ -47,7 +47,8 @@ export default defineConfig({
     Icons({
       compiler: 'vue3',
       iconCustomizer(collection, icon, props) {
-        props.class = 'text-sm';
+        props.width = '1em';
+        props.height = '1em';
       },
       customCollections: {
         c: FileSystemIconLoader('./src/assets/icons', svg =>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice, yeah that's cleaner! But now all the other icons are 18px instead of 20px, the only solution I found for this is changing the global text size in App.vue from base to md. Or we could add text-md to each icon.

},
customCollections: {
c: FileSystemIconLoader('./src/assets/icons', svg =>
svg.replace(/^<svg /, '<svg fill="currentColor" ')
)
}
})
],
Expand Down
Loading