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

famfamfam sprites generation tool #692

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

abetis
Copy link
Contributor

@abetis abetis commented Apr 15, 2018

Script that combines all the PNGs from src/themes/icons/src/famfamfam into a single PNG sprite file and generates the famfamfam.less according to the icons order.
grip.png and grip-rtl.png are added at the end.

Don't see any easier way to add new icons.
Sorry Windoze guys, works only on Linux, don't know what tools do you have in the MS world.
Required ImageMagick installed.

Copy link
Collaborator

@brunoais brunoais left a comment

Choose a reason for hiding this comment

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

I dunno if @samclarke would accept this but, for me to accept (see below), it has to work on all OSs and, preferably, not introduce a new programming language.

@@ -0,0 +1,48 @@
#!/bin/bash
Copy link
Collaborator

Choose a reason for hiding this comment

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

Change the script language into a platform-agnostic one. For the sake of this project's survival we must not limit image contributions to linux users (even if only through an emulator or whatnot).

Because, to build, SCE already requires node.js (javascript), I highly recommend you using that. It will allow us to, more easily, create a workflow including this process and without introducing a new language.
imagemagick exists for all 3 major platforms so that is not an issue.

Take a look at this. It might help you getting started on the communications part.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, but I will leave it to someone else.
As I intend to use that script for my next PR (facebook video) and other changes specific to my system that won't be pushed, I propose to change the order of the icons in the sprite image to be in alphanumeric order and not sporadic as it is now and change the url.png to link.png to have all the icons match the command names.
I can push those changes separately.

Since there is no tool at all to create the sprite image and it have to be done manually, I don't see a reason why not to use it until there will be a better solution proposed by someone else.
I don't intend to create the image manually for my next changes. Any merge in the image will not be possible without a tool.

Copy link
Collaborator

@brunoais brunoais Apr 15, 2018

Choose a reason for hiding this comment

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

Please activate to allow changes from maintainers
image
^

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it is allowed on my side...

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks.

@brunoais brunoais requested a review from samclarke April 15, 2018 10:17
@brunoais
Copy link
Collaborator

brunoais commented Apr 15, 2018

@samclarke I checked famfamfam's licence and this script is allowed. Is it good to go after transforming it into javascript for Node.js?

@abetis
Copy link
Contributor Author

abetis commented Apr 15, 2018

Ohhh... now I see that there is a website with all the famfamfam icons...
What should I do if I want to add a facebook icon to the color icons set, but there is no such icon on the famfamfam's page? At least I couldn't find it...

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.

2 participants