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

Adding 19 new icons. #2186

Closed
wants to merge 1 commit into from
Closed

Adding 19 new icons. #2186

wants to merge 1 commit into from

Conversation

duese
Copy link
Contributor

@duese duese commented Jul 13, 2024

No description provided.

@Donnnno
Copy link
Collaborator

Donnnno commented Jul 17, 2024

Hi, looks like you removed the entire appfilter! You must only add yours to the top, bottom, basically anywhere, but don't override the other lines 🙃

@Donnnno
Copy link
Collaborator

Donnnno commented Jul 17, 2024

can you rebase with the main branch here and add your icons again? I can't check it out due to the many changes

@duese
Copy link
Contributor Author

duese commented Jul 18, 2024

can you rebase with the main branch here and add your icons again? I can't check it out due to the many changes

Sorry, I messed up when pushing the branch. It should be fixed now.

@Donnnno
Copy link
Collaborator

Donnnno commented Jul 18, 2024

Hi, thanks!
I've taken a look at your icons, great work!
I'm adding them in a fresh pr (for some reason I can't open this one with GitHub desktop)

Here are some things I noticed:

  • make sure to use the updated basic square icon, it has slightly rounder corners, also the text was really weird here:
    image

Donnnno added a commit that referenced this pull request Jul 18, 2024
@Donnnno Donnnno closed this Jul 18, 2024
@duese
Copy link
Contributor Author

duese commented Jul 18, 2024

Thanks for the hint. Had to run the SVGs through several tools to get the discouraged SVG elements removed as stated in the docs. Maybe they got crumbled in the process of doing so.

@Donnnno
Copy link
Collaborator

Donnnno commented Jul 18, 2024

Oh don't worry about the transform elements, I'll take care of that stuff, when you upload them :-)

@duese
Copy link
Contributor Author

duese commented Jul 18, 2024

Okay, good to hear. 😃 But maybe your workload could be reduced if there's a section in the documentation explaining how to get rid of the transform elements in the first place.

@Donnnno
Copy link
Collaborator

Donnnno commented Jul 18, 2024

I think it can be removed from the contribution guide altogether, we used an old export method that didn't support it, but the newer one should have the functionality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants