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

Removed overlapping line in align-horizontal-distribute-end #1608

Closed
wants to merge 1 commit into from

Conversation

jguddas
Copy link
Member

@jguddas jguddas commented Oct 14, 2023

What is the purpose of this pull request?

  • New Icon
  • Bug fix
  • New Feature
  • Documentation update
  • Other: Optimization

Let me ask, should we have overlapping lines like we have in icons like this?

@github-actions github-actions bot added the 🎨 icon About new icons label Oct 14, 2023
@github-actions
Copy link

Added or changed icons

icons/align-horizontal-distribute-end.svg

Preview cohesion icons/arrow-down-square.svg
icons/align-horizontal-distribute-end.svg
icons/heading-2.svg
Preview stroke widths icons/align-horizontal-distribute-end.svg
icons/align-horizontal-distribute-end.svg
icons/align-horizontal-distribute-end.svg
DPI Preview (24px) icons/align-horizontal-distribute-end.svg
Icon X-rays icons/align-horizontal-distribute-end.svg

@@ -9,8 +9,8 @@
stroke-linecap="round"
stroke-linejoin="round"
>
<rect width="6" height="14" x="4" y="5" rx="2" />
<rect width="6" height="10" x="14" y="7" rx="2" />
<path d="M10 17a2 2 0 0 1-2 2H6a2 2 0 0 1-2-2V7a2 2 0 0 1 2-2h2a2 2 0 0 1 2 2" />
Copy link
Member

Choose a reason for hiding this comment

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

Why should we go for more code?

Copy link
Member Author

Choose a reason for hiding this comment

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

This removes two places where elements are on top of each other, it's imo okay to have that.

Just wanted to know what everyone else is thinking about overlapping lines like we have here.

@jguddas
Copy link
Member Author

jguddas commented Nov 14, 2023

Honestly, probably easier to just start flattening path to resolve issues like this.

image

Copy link

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the Stale label Dec 15, 2023
@ericfennis
Copy link
Member

@jguddas Overlapping is currently happening with a lot of icons right now. I agree there is an improvement to be made to support opacity colors like rgba in a better way. But other than that I don't see any problems with overlapping of SVG elements.

The idea of merging paths is something we need to research, I prefer to do this on built time. this is added to the list on Lucide v1, see issue #1687.

I'm not in favor of fixing this individually, so I close this one for now

@ericfennis ericfennis closed this Dec 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎨 icon About new icons Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants