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

update icon: filezilla (original, original-wordmark, plain, plain-wordmark, line, line-wordmark) #2051

Merged
merged 7 commits into from
Dec 22, 2023

Conversation

weh
Copy link
Contributor

@weh weh commented Nov 24, 2023

Double check these details before you open a PR

  • PR does not match another non-stale PR currently opened
  • PR name matches the format new icon: Icon name (versions separated by comma). More details here
  • PR's base is the develop branch.
  • Your icons are inside a folder as seen here
  • SVG matches the standards laid out here
  • A new object is added in the devicon.json file as seen here
  • Updated existing SVG (removed padding, optimized size)
  • Added line versions

TBH i am not sure if the Wordmark version is official. I also did not find a suitable original-wordmark file, and the plain-wordmark looks a bit unclean.

This PR closes NONE

Related to #1998

Link to prove your SVG is correct and up-to-date.

@weh weh requested review from a team, ConX, Snailedlt, canaleal and lunatic-fox and removed request for a team November 24, 2023 13:40
@weh weh changed the title update icon filezilla (original, plain, plain-wordmark, line, line-wordmark) update icon: filezilla (original, plain, plain-wordmark, line, line-wordmark) Nov 24, 2023
Copy link
Collaborator

@Snailedlt Snailedlt left a comment

Choose a reason for hiding this comment

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

The icons look good visually, just the code that needs some more love :)
overall good work, almost done!

icons/filezilla/filezilla-plain.svg Outdated Show resolved Hide resolved
icons/filezilla/filezilla-plain-wordmark.svg Outdated Show resolved Hide resolved
icons/filezilla/filezilla-line-wordmark.svg Outdated Show resolved Hide resolved
icons/filezilla/filezilla-line.svg Outdated Show resolved Hide resolved
@Snailedlt Snailedlt added the feature:icon Use this label for pull requests when a new icon is ready to be added to the collection label Nov 25, 2023
Copy link
Contributor

@lunatic-fox lunatic-fox left a comment

Choose a reason for hiding this comment

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

Besides @Snailedlt pointed out, this icon has some particular issues:

  • There is no original-wordmark version to plain-wordmark exist and original-wordmark can't be aliased to it's plain version.
  • original-wordmark should be following this image below, unfortunately is a png and the text have some soft gradient.

I think "The free FTP solution" text can be omitted.

  • original should be following the original icon with the Fz styled letters with #ffffff background.

I'll be trying to do the mentioned changes later today.

@lunatic-fox
Copy link
Contributor

Besides @Snailedlt pointed out, this icon has some particular issues:

  • There is no original-wordmark version to plain-wordmark exist and original-wordmark can't be aliased to it's plain version.
  • original-wordmark should be following this image below, unfortunately is a png and the text have some soft gradient.

I think "The free FTP solution" text can be omitted.

  • original should be following the original icon with the Fz styled letters with #ffffff background.

I'll be trying to do the mentioned changes later today.

Suggestions

original-wordmark- no text gradient
original
plain-wordmark
plain
line-wordmark
line

@weh weh changed the title update icon: filezilla (original, plain, plain-wordmark, line, line-wordmark) update icon: filezilla (original, original-wordmark, plain, plain-wordmark, line, line-wordmark) Dec 6, 2023
@weh
Copy link
Contributor Author

weh commented Dec 6, 2023

many thanks to @lunatic-fox i added your images and updated the PR

Soft gradient added to text.
@lunatic-fox
Copy link
Contributor

many thanks to @lunatic-fox i added your images and updated the PR

Thank you too @weh!
I made the little change on original-wordmark since I figured out how to do the inset gradient on text.

Copy link
Collaborator

@Snailedlt Snailedlt left a comment

Choose a reason for hiding this comment

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

Looks great for the most part, just some minor color discrepancies that need fixing before we're good to merge.

Previews shouldn't change from these suggestions, so I'll post them here:
image
image
image
image
image

icons/filezilla/filezilla-plain.svg Outdated Show resolved Hide resolved
devicon.json Outdated Show resolved Hide resolved
@Snailedlt
Copy link
Collaborator

@lunatic-fox please take another look 🙇‍♂️

@Snailedlt Snailedlt merged commit b9a0e0b into devicons:develop Dec 22, 2023
5 checks passed
@Snailedlt Snailedlt mentioned this pull request Feb 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature:icon Use this label for pull requests when a new icon is ready to be added to the collection
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants