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

feat: Add SoundCloud icon #603

Closed

Conversation

shawnwwwww
Copy link

followed guideline from #171

followed guideline from feathericons#171
Copy link

@moeenio moeenio left a comment

Choose a reason for hiding this comment

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

Remove , <title>, , and other not useful tags. remove ids. Just keep the necessary svg. You can see other icons's svg code to get an idea of how it should be.

@codecov
Copy link

codecov bot commented May 23, 2019

Codecov Report

Merging #603 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #603   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           5      5           
  Lines          34     34           
  Branches        3      3           
=====================================
  Hits           34     34

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5a991dd...90940d7. Read the comment docs.

@moechaieb
Copy link

LGTM. I'll let @locness3 approve though :)

@shawnwwwww
Copy link
Author

LGTM. I'll let @locness3 approve though :)

idk i need to clean the svg code as well so it might takes some time

@shawnwwwww
Copy link
Author

Remove , <title>, , and other not useful tags. remove ids. Just keep the necessary svg. You can see other icons's svg code to get an idea of how it should be.

i've compared a few official icons and cleaned it up :)

Copy link

@moeenio moeenio left a comment

Choose a reason for hiding this comment

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

Now please remove line breaks for svg tag

@colebemis
Copy link
Member

Now please remove line breaks for svg tag

@locness3 Line breaks are okay. The build script actually uses prettier to format the SVGs 😄

@shawnwwwww It looks like there might be a viewBox issue. This is how GitHub renders the SVG:

image

@moeenio
Copy link

moeenio commented Jun 5, 2019

@locness3 Line breaks are okay. The build script actually uses prettier to format the SVGs 😄

Oh ok sorry

@colebemis
Copy link
Member

@locness3 No need to apologize 😄 I really appreciate all your help!

@mittalyashu
Copy link
Contributor

@shawnwwwww Can you fix the issue?

Change the PR title to Add SoundCloud icon

@moeenio
Copy link

moeenio commented Jul 22, 2019

Change the PR title to Add SoundCloud icon

Or feat: Add SoundCloud icon

@shawnwwwww shawnwwwww changed the title added soundcloud icon feat: Add SoundCloud icon Jul 22, 2019
@shawnwwwww
Copy link
Author

Change the PR title to Add SoundCloud icon

Or feat: Add SoundCloud icon

changed!

@shawnwwwww
Copy link
Author

@shawnwwwww Can you fix the issue?

Change the PR title to Add SoundCloud icon

changed to feat: Add SoundCloud icon

@mittalyashu
Copy link
Contributor

While previewing the icon it is showing.

image

🤔

@jletey
Copy link

jletey commented Jul 22, 2019

@shawnwwwww Change the icon to this so that the icon completely fits in the 24x24px grid

Shot 2019-07-22 at 12 35 13

<svg
  width="24"
  height="24"
  viewBox="0 0 24 24"
  xmlns="http://www.w3.org/2000/svg"
  fill="none"
  stroke="currentColor"
  stroke-width="2"
  stroke-linecap="round"
  stroke-linejoin="round"
>
  <path
    clip-rule="evenodd"
    d="M13 17.236h7.3C22.096 17 23 16.084 23 14.488c0-1.596-.904-2.457-2.7-2.583h-.338c0-1.974-.866-3.409-2.598-4.305-1.732-.897-2.716-.788-4.364.327v9.31z"
  />
  <path d="M10 8.766v8.47" />
  <path d="M7 10.178v7.058" />
  <path d="M4 12.295v4.941" />
  <path d="M1 14.354v2.47" />
</svg>

@moeenio
Copy link

moeenio commented Jul 22, 2019

I think you should keep some spacing @johnletey

@jletey
Copy link

jletey commented Jul 22, 2019

@locness3 There is a 1px space between everything

@moeenio
Copy link

moeenio commented Jul 22, 2019

Between the borders of the icon

@jletey
Copy link

jletey commented Jul 22, 2019

@locness3 I've added 1px between the borders of the icon

Shot 2019-07-22 at 16 35 00

@shawnwwwww Use this:

<svg
  width="24"
  height="24"
  viewBox="0 0 24 24"
  xmlns="http://www.w3.org/2000/svg"
  fill="none"
  stroke="currentColor"
  stroke-width="2"
  stroke-linecap="round"
  stroke-linejoin="round"
>
  <path d="M8 8.766v8.47" />
  <path d="M5 10.178v7.058" />
  <path d="M2 12.295v4.941" />
  <path d="M11 16.973l7.175.027h.668C20.924 17 22 15.539 22 14.084c0-1.454-1.119-2.741-3.157-2.741h-.668V9.76c0-1.191-.58-2.935-2.625-3.56-2.044-.627-4.55.248-4.55 2.187v8.585z" />
</svg>

@moeenio
Copy link

moeenio commented Oct 19, 2019

@shawnwwwww Could you update your icon ?

@colebemis
Copy link
Member

Thanks for opening this PR, @shawnwwwww! But I'm going to close this because we've decided to stop adding logo icons to Feather (see #763 for more details).

@colebemis colebemis closed this Apr 4, 2020
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.

6 participants