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 grunt-spritesmith.js #165

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

billymoon
Copy link

allow config object sortFn to optionally sort icons accorging to custom logic before rendering

usage: (e.g. to sort in random order...)

{
    sortFn: function(a, b){
      return Math.random() < 0.5 ? -1 : 1;
    },
    src: ['icon-arrow.png', 'icon-circle.png'],
    dest: 'img/icons/sprite.png'
}

allow config object `sortFn` to optionally sort icons accorging to custom logic before rendering
@twolfson
Copy link
Owner

Can you explain your use case for this more?

@billymoon
Copy link
Author

@twolfson we are removing ruby dependency on a project, and are trying to swap out old build jobs for new, but with same output. Our build job needs to select order of sprites so that css output is same without needing to change content (which is not really feasible for us). We have icons of multiple sizes, and have class scheme where css order is important, something like...

.icon.icon-warning {
    background-image: url('data:/*some svg path data*/');
    width: 12px;
    height: 12px;
}

.icon.icon-warning.icon-warning-large {
    width: 24px;
    height: 24px;
}

... and in html ...

<!-- small icon -->
<i class="icon icon-warning"></i>
<!-- large icon -->
<i class="icon icon-warning icon-warning-large"></i>

The order of the rules is important, as the size of the last one will override the earlier ones. We output SVG for most browsers, and use sprite as fallback, so need extra rules for offset added for fallback.

We use the callback to sort by an array of files we manually updated, which works fine for our needs.

@twolfson
Copy link
Owner

Input sprite order can be determined by using an array instead of a glob. I think that should work for your use case:

src: ['icon-warning.png', 'icon-warning-large.png']

@twolfson twolfson closed this Aug 12, 2016
@billymoon
Copy link
Author

@twolfson nope - tried that.

Looks like the array is being sorted alphabetically before being added to cleanCoords.

https://github.com/Ensighten/grunt-spritesmith/blob/master/src/grunt-spritesmith.js#L210

Our sort function actually just compares position in the array we provide to src to return to the original order we supplied.

@twolfson
Copy link
Owner

Ah, maybe we should add a boolean flag to enable/disable sorting (true by default for backwards compatibility):

sort: false,
src: ['icon-arrow.png', 'icon-circle.png'],
dest: 'img/icons/sprite.png'

@twolfson twolfson reopened this Aug 13, 2016
@DawidMyslak
Copy link

@twolfson is there any chance to approve this PR?

I have exactly the same issue, so order of generated CSS classes is important for me, but at the moment there is no way to control it.

@billymoon
Copy link
Author

@twolfson what do you say to this PR..?

@twolfson
Copy link
Owner

I never heard a reply to the boolean flag counterpoint =/ I think it'd be good to avoid one-more function in our setup if possible

@billymoon
Copy link
Author

I don't see value in flag. Current order is unpredictable, determined by operating system order of files returned (I think). Always sorting makes it predictable which is useful feature in my book. Customising sort function allows for lots of control over order. If you need to base order on something other than the name, you can use a custom shirt function to compare against some other criteria about your icons.

@twolfson
Copy link
Owner

The order is set by whatever is passed into Grunt as its src. We happen to perform an additional sort currently but I considered this was a PR to disable/workaround that =/

https://github.com/Ensighten/grunt-spritesmith/blob/6.6.0/src/grunt-spritesmith.js#L210

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.

3 participants