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

Feature: Add pin and pin-vertical #626

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

junipf
Copy link

@junipf junipf commented Jun 21, 2019

pin
pin-vertical
Used guidelines in #171
Fixes #407

@codecov
Copy link

codecov bot commented Jun 21, 2019

Codecov Report

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

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #626   +/-   ##
=====================================
  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 3c64e93...d57e7f5. Read the comment docs.

@jletey jletey mentioned this pull request Jun 23, 2019
@MarcelloTheArcane
Copy link
Contributor

Could you try it with the tops slightly narrower? Just an idea 🙂

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.

Maybe renaming to pin-2 ?

@mittalyashu
Copy link
Contributor

mittalyashu commented Jul 22, 2019

@junipf Keep the vertical pin name as pin and change the tilted pin name to pin-tilted.
What do you think? @locness3

The size of this titled pin is bigger as compared to the vertical pin.

image

@moeenio
Copy link

moeenio commented Jul 22, 2019

pin-titled ?

@mittalyashu
Copy link
Contributor

Sorry, I mean pin-tilted. 😅

@moeenio
Copy link

moeenio commented Jul 22, 2019

Oh ok.

@junipf
Copy link
Author

junipf commented Jul 23, 2019

The size of this titled pin is bigger as compared to the vertical pin.

They're sized to have at least 1px between the icon and the edge of the canvas per the guidelines. So long as they aren't used together (which they reasonably shouldn't be), I think this is preferable.

I'm fine with any renaming

@ahtohbi4
Copy link
Contributor

For my project, I have made the other ones:

Снимок экрана 2019-07-24 в 11 46 48

Снимок экрана 2019-07-24 в 11 55 33

But yours are nice too. ☺️

@moeenio
Copy link

moeenio commented Jul 24, 2019

They look a lot detailed

@mittalyashu
Copy link
Contributor

Let's add all of them.

Just to keep the shape of the pin consistent, can some copy the pinhead from an icon to another.

@MarcelloTheArcane
Copy link
Contributor

@ahtohbi4 Could you increase the gap of the top ridge?

@ahtohbi4
Copy link
Contributor

Let's add all of them.

@mittalyashu I don't think that it is a good idea to store a million kind of the same icons. It will turn a collection of good icons into a dump. 😌

@ahtohbi4 Could you increase the gap of the top ridge?

@MarcelloTheArcane I can give you SVG-code of my variants if you'd like.

@mittalyashu
Copy link
Contributor

@mittalyashu I don't think that it is a good idea to store a million kind of the same icons. It will turn a collection of good icons into a dump. 😌

But I love both icons 😭.

@junipf
Copy link
Author

junipf commented Jul 24, 2019

Those are really great @ahtobi4! I think the head shape on yours are a lot more readable than mine, but they might be a bit noisey, so I'd love to play around with them and hopefully sort of merge our designs if you wouldn't mind.

@ahtohbi4
Copy link
Contributor

so I'd love to play around with them and hopefully sort of merge our designs if you wouldn't mind.

@junipf sure 🙂

Here is the code:

<svg
  xmlns="http://www.w3.org/2000/svg"
  width="24"
  height="24"
  viewBox="0 0 24 24"
  fill="none"
  stroke="currentColor"
  stroke-width="2"
  stroke-linecap="round"
  stroke-linejoin="round"
>
  <path d="M15 5a2 2 0 0 0 2-2V2H7v1c0 1.1.9 2 2 2m6 0l1 7m-1-7H9m7 7H8m8 0s3 .5 3 4H5c0-3.5 3-4 3-4m0 0l1-7M12 16v6" />
</svg>
<svg
  xmlns="http://www.w3.org/2000/svg"
  width="24"
  height="24"
  viewBox="0 0 24 24"
  fill="none"
  stroke="currentColor"
  stroke-width="2"
  stroke-linecap="round"
  stroke-linejoin="round"
>
  <path d="M17.85 10.2l-4.24 5.65m4.24-5.66L13.6 5.95m4.24 4.24a2 2 0 0 0 2.83 0l.7-.7-7.07-7.08-.7.71a2 2 0 0 0 0 2.83m0 9.9l-5.66-5.66m5.66 5.66s1.76 2.47-.71 4.95L3 10.9c2.47-2.48 4.95-.7 4.95-.7m0 0l5.66-4.25M7.95 15.85l-4.24 4.24" />
</svg>

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.

Icon Request: Pin
5 participants