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

Use current-color in svgs #262

Merged
merged 2 commits into from
Dec 14, 2023
Merged

Use current-color in svgs #262

merged 2 commits into from
Dec 14, 2023

Conversation

SimonSimCity
Copy link
Collaborator

Avoid the usage of css to define a color on an svg image, but rather in SVG use currentColor.

Copy link

codecov bot commented Dec 6, 2023

Codecov Report

Attention: 13 lines in your changes are missing coverage. Please review.

Comparison is base (66fa354) 10.91% compared to head (5746176) 10.70%.
Report is 19 commits behind head on main.

Files Patch % Lines
components/track/TrackItem.vue 0.00% 3 Missing ⚠️
components/ProfileMenu.vue 0.00% 2 Missing ⚠️
modules/icons/runtime/components/nuxt-icon.vue 0.00% 2 Missing ⚠️
components/MediaPlayer.vue 0.00% 1 Missing ⚠️
components/SiteLogo.vue 0.00% 1 Missing ⚠️
components/album/AlbumItem.vue 0.00% 1 Missing ⚠️
components/contributor/ContributorListItem.vue 0.00% 1 Missing ⚠️
components/playlist/PlaylistItem.vue 0.00% 1 Missing ⚠️
components/podcast/PodcastItem.vue 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #262      +/-   ##
==========================================
- Coverage   10.91%   10.70%   -0.21%     
==========================================
  Files          95       97       +2     
  Lines        4306     4389      +83     
  Branches      179      181       +2     
==========================================
  Hits          470      470              
- Misses       3746     3827      +81     
- Partials       90       92       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

github-actions bot commented Dec 6, 2023

Azure Static Web Apps: Your stage site is ready! Visit it here: https://kind-meadow-01b17e903-262.westeurope.3.azurestaticapps.net

@SimonSimCity
Copy link
Collaborator Author

In the preview I could see that the following images have a change:

Fill:

  • assets/icons/download.svg
  • assets/icons/icon.loading (animation).svg
  • assets/icons/queue.svg

Color:

  • assets/icons/nav.browse.svg
  • assets/icons/nav.favorites.svg
  • assets/icons/nav.home.svg
  • assets/icons/nav.ios.browse.svg
  • assets/icons/nav.ios.favorites.svg
  • assets/icons/nav.ios.home.svg
  • assets/icons/nav.ios.library.svg
  • assets/icons/nav.ios.profile.svg
  • assets/icons/nav.ios.search.fill.svg
  • assets/icons/nav.library.svg
  • assets/icons/nav.profile.svg
  • assets/icons/nav.search.svg

I was also surprised how many duplicates we actually have among those images... @sclausen is there a common procedure of extracting those images from Figma, or is there a reason why so many images are duplicated?

Copy link
Collaborator

@sifferhans sifferhans left a comment

Choose a reason for hiding this comment

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

Looks good! There are a couple of icons with unnecessary fills though:

  • download.svg
  • icon.loading (animation).svg

Other than that, looks good!

@SimonSimCity
Copy link
Collaborator Author

Fills are fixed now 😎👍

Colors, which I lined out as change should be changed via CSS.

Copy link

github-actions bot commented Dec 7, 2023

Azure Static Web Apps: Your stage site is ready! Visit it here: https://kind-meadow-01b17e903-262.westeurope.3.azurestaticapps.net

Copy link

github-actions bot commented Dec 7, 2023

Azure Static Web Apps: Your stage site is ready! Visit it here: https://kind-meadow-01b17e903-262.westeurope.3.azurestaticapps.net

Copy link
Collaborator

@sifferhans sifferhans left a comment

Choose a reason for hiding this comment

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

These still don't look right to me

CleanShot 2023-12-13 at 15 55 59

CleanShot 2023-12-13 at 15 56 18

@sclausen
Copy link

@SimonSimCity are you really referring to me? I have to admit that I have absolutely no memory participating in this project at all.

@SimonSimCity
Copy link
Collaborator Author

SimonSimCity commented Dec 13, 2023

@sclausen sorry for pulling you in here, I had a typo in the username 🙈 I meant to ping @sclausendk

@SimonSimCity
Copy link
Collaborator Author

@sifferhans the two files you're referring to are not part of the changed files anymore - they're just part of individual commits 🤗

@sifferhans
Copy link
Collaborator

@SimonSimCity Ah, right! Then everything looks good to me 😁

@SimonSimCity SimonSimCity merged commit 19961d4 into main Dec 14, 2023
8 of 10 checks passed
@SimonSimCity SimonSimCity deleted the use-current-color-svg branch December 14, 2023 10:33
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.

3 participants