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

Fix HiDPI CSS dimensions, make the selector option more powerful (fixes #17) #18

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

Conversation

chearon
Copy link
Contributor

@chearon chearon commented Mar 8, 2013

The 2 most recent commits simply fix HiDPI so that on retina devices, the images arent twice the size they're supposed to be.

The 3rd commit was discussed on #9 (you didn't seem convinced, but hear me out). I wish I'd developed them separate branches, because the retina patches are what really need to be upstream. But essentially, this makes the selector option more intelligent.

Example: selector option is set to .spriticon and spritesheet calls resolveImageSelector on a file which returns .icon-outer .file. It only makes sense to generate the selector .icon-outer .spriticon.file.

It essentially does the same thing it used to except that it knows to apply the rule to the last selector in a space-separated hierarchy. It also expects resolveImageSelector to return a real selector, rather than assuming it to just be a single class name.

Anyway, take it or leave it. I found it useful and sensible. Thanks!

@mente
Copy link

mente commented Mar 8, 2013

You can create new branch and merge changes from upstream. Then apply retina related commits via git diff && git apply. This way you can separate commits instead of pushing all of them.

@mente
Copy link

mente commented Mar 8, 2013

Forgot to mention: drop own commits via git reset --hard before merging upstream. And only then apply them manually. Hope this helps to get this PR faster in upstream

@chearon
Copy link
Contributor Author

chearon commented Mar 8, 2013

Yeah, crap, everything before the merge is irrelevant. I just used git checkout -b retina-fixes butler/master to start a new branch that's at the same HEAD of the master here...does that achieve the same effect? Then I'll do the manual commits...guess I should start developing features on different branches

@richardbutler
Copy link
Owner

@chearon Apologies for the delay on this one. It would introduce a breaking change, so would push onto 0.5, but if you want to add changes to the readme and example, I'll happily look into it a bit more as you make a good case for your use case.

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