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

Slimmage Background Image plugin #57

Open
wants to merge 4 commits into
base: unstable
Choose a base branch
from

Conversation

protherj
Copy link

@protherj protherj commented Nov 6, 2015

This is my first attempt at a Background Images plugin based on the Pull Request #37 from dancek. Slimmage has changed considerably and I hope I got it generally right.

Note: see the thread for Pull Request #55 for more background.

@protherj
Copy link
Author

protherj commented Nov 6, 2015

How do I fix the Travis CI build issues?

@erikbra
Copy link

erikbra commented Nov 12, 2015

Looks like performance issues, if I interpret the log correctly (red timings), or am I wrong?

@lilith
Copy link
Member

lilith commented Nov 12, 2015

First we need to fix the existing problems. Then we can address this PR by adding associated tests for the new functionality.

@wiedikerli
Copy link

@nathanaeljones whats the status of this pr?

@ProNotion
Copy link

This is working great for me so far - thanks Jason. The only issue of slight concern is the lack of fallback like we have with elements on images but I'm not sure how we could achieve this with background images in a nice clean way.

@wiedikerli
Copy link

@nathanaeljones any plans to merge this pr?

@firthy
Copy link

firthy commented Sep 22, 2016

@nathanaeljones I'd like to see this PR as part of the plugin also

@lilith
Copy link
Member

lilith commented Sep 22, 2016

Slimmage.js tries to do a very difficult thing. 99% of possible techniques fail on at least 1 crucial browser. This is not a space where I can merge pull requests for features which don't have associated unit tests. The chance of any new feature working across the same set of browsers is miniscule, in general.

I can't merge this without tests, and I don't have the resources to fix the bitrot in the existing tests. If someone wants to tackle #58, I hand out write access quite readily. Once #58 is fixed, then tests can be added to this PR and (if they pass), it can be merged.

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.

None yet

6 participants