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: defention of done #1224

Merged
merged 19 commits into from
Aug 21, 2023
Merged

Conversation

jguddas
Copy link
Member

@jguddas jguddas commented May 6, 2023

  • :octocat: = Reviewer
  • 🐙 = Contributor
  • 🤖 = GitHub workflow

I have thought of everything:

  • Elements have at least 1 pixel padding within the canvas. (:robot:)
  • Icons are visually centered. (:octocat: and maybe :robot: https://github.com/mapbox/polylabel)
  • Icons have a similar optical volume as circle and square. (:octocat: and maybe :robot: with convex hull)
  • Icons have similar visual density and level of detail. (:octocat: and maybe :robot:, could just use total line length)
  • Icons are fun and playful. (:octopus: & :octocat:)
  • Elements and Arcs centers align with the grid. (:robot:)
  • Corners have a 2px radius or have a good reason for sharpness. (:octopus:)
  • Holes and gaps only become invisible with stroke widths of at least 4px. (:robot:)
  • Gaps are closed, so they are 2px when possible.(:octocat:) feat: defention of done #1224 (comment)
  • Curves join smoothly. (:octocat:) https://www.figma.com/community/plugin/932729267119671976/Curvature
  • Icon is at most based on other lucide icon and not on other people's work (:octopus:)
  • Icon should not be a rotated or mirrored version of an already existing icon (:octopus: & :octocat:)
  • Icons that are part of a group are named group-type so, for example, badge-plus is based on badge.
  • Icons are named by what they are rather than their use case, for example save should rather be named to floppy-disk.
  • Icons do not end with a number like -2, -3, -4, etc.
  • Icons do not represent a brand or logo
  • Icons are sharp on low dpi displays
  • Icons have a specific use case that is not currently covered by another icon (please explain what this icon is used for, the more use cases the better)
  • Icons do not include religious or political imagery
  • Icons do not contain hate symbols
  • Reuse shapes cross icons

I have optimized everything:

  • Arc path segments are used in place of curves when possible. (:octocat:, should be :robot:, sadly currently flaky)
  • circle and rect are used instead of path when possible. (:robot:)
  • path is used instead of polyline, line and polygon. (:robot:)
  • Paths only contain one move segment. (:robot:)
  • Dots use M{{x}} {{y}}h0.01. (:robot:)

@vercel
Copy link

vercel bot commented May 6, 2023

@jguddas is attempting to deploy a commit to the Lucide Team on Vercel.

A member of the Team first needs to authorize it.

@jguddas jguddas force-pushed the feat/definition-of-done branch 2 times, most recently from d26b378 to 1f3359a Compare May 6, 2023 14:44
@jguddas jguddas marked this pull request as draft May 6, 2023 14:45
@jguddas
Copy link
Member Author

jguddas commented May 7, 2023

What about making closing this kind of gap a guideline @danielbayley @karsa-mistmere @ericfennis ?

simplescreenrecorder-2023-05-07_15.21.47.mp4

@jguddas
Copy link
Member Author

jguddas commented May 8, 2023

We should also add some rules for arrows.

@karsa-mistmere
Copy link
Member

karsa-mistmere commented May 8, 2023

Thorough as this is, I feel like it might disencourage and alienate a number of potential future contributors who might be really good at design but not expert at writing SVG code. 🤔

Most of these parts could be repurposed as a reviewers' checklist maybe?

@jguddas
Copy link
Member Author

jguddas commented May 8, 2023

Good point, we should split things up into:

  • Checklist for reviewer.
  • Checklist for contributor.
  • Checks we automate and put into a GitHub workflow.

Edit: added some annotations to the PR description.

@jguddas
Copy link
Member Author

jguddas commented Jun 3, 2023

Variations of icons are named name-variation so, i.e. badge-plus badge-plus is a variation of the badge icon.

@airone01
Copy link
Contributor

airone01 commented Jun 5, 2023

@jguddas we might want to debate adding the two rules from #1336 here. I'll remove my issue so it doesn't bloat the issue list ❤️.
The two rules are:

@danielbayley
Copy link
Member

  • M{{x}} {{y}}h0.01. (🤖)

@jguddas Shouldn’t that be M{{x}} {{y}}h.01?

@lucide-icons lucide-icons deleted a comment from github-actions bot Aug 16, 2023
@lucide-icons lucide-icons deleted a comment from vercel bot Aug 16, 2023
@jguddas jguddas marked this pull request as ready for review August 16, 2023 19:42
.github/pull_request_template.md Outdated Show resolved Hide resolved
.github/pull_request_template.md Outdated Show resolved Hide resolved
.github/pull_request_template.md Outdated Show resolved Hide resolved
.github/pull_request_template.md Outdated Show resolved Hide resolved
.github/pull_request_template.md Outdated Show resolved Hide resolved
.github/pull_request_template.md Outdated Show resolved Hide resolved
.github/pull_request_template.md Outdated Show resolved Hide resolved
.github/pull_request_template.md Show resolved Hide resolved
.github/pull_request_template.md Outdated Show resolved Hide resolved
.github/pull_request_template.md Outdated Show resolved Hide resolved
.github/pull_request_template.md Outdated Show resolved Hide resolved
@ericfennis ericfennis merged commit e78d910 into lucide-icons:main Aug 21, 2023
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.

5 participants