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

Two Improvements #29

Closed
tomasdev opened this Issue Jul 10, 2012 · 10 comments

Comments

Projects
None yet
3 participants
@tomasdev

tomasdev commented Jul 10, 2012

  1. Considering the relevant data attribute is data-picture, we could not force the tag to be always <div> in which case, we could use the <picture> element.
  2. Considering anchor tags (<a>), I have tried a bunch of combinations without success to make the image linkable. Should we define a better way? Like data-href perhaps so the <img> appended is wrapped in <a> tag ?
@scottjehl

This comment has been minimized.

Owner

scottjehl commented Jul 10, 2012

Thanks.

  1. Using the picture element is probably not a safe idea at this time, given that the specs are in flux. Also, that element requires lots of workarounds that we avoid with divs, such as shimming for IE support, and also having to duplicate all source elements with comments because IE9 can't support them within an unknown element. For those reasons, I moved this repo away from using picture until the specification dust settles. That said, using a span may be useful for inline images. That change is already under consideration.
  2. Did you try wrapping an a around the div[data-picture] element?
@tomasdev

This comment has been minimized.

tomasdev commented Jul 10, 2012

  1. Using picture is up to the developer. My idea is not just querying by tagName. Cool answer!
  2. I am currently using that, but I don't feel quite comfortable with that and old IE, even thou it's on HTML5 to be able to do <a><div></div></a>
@scottjehl

This comment has been minimized.

Owner

scottjehl commented Jul 10, 2012

  1. Of course, but the script no longer contains the workarounds to make picture work, so it'd definitely break. I'm merely suggesting that opening up the selector would not allow picture to be used safely. Anyway, to your broader suggestion: querying all elements would slow down the script overall, as I've got it wired up to use more widely-supported DOM lookups (getElementsByTagName) rather than querySelectorAll. Is that the change you were hoping to see? If you can submit a pull that makes this change without narrowing browser support or noticeably slowing things down, I'd be happy to consider pulling it in.
  2. Is there a particular issue with old IE and a div? I'm not familiar, and it'd be great to know.
@tomasdev

This comment has been minimized.

tomasdev commented Jul 10, 2012

w.addEventListener( "resize", w.picturefill, false );

Sorry by commenting this here, but... do you have any particular reason why it's not throttled ?

@scottjehl

This comment has been minimized.

Owner

scottjehl commented Jul 10, 2012

There's a bug for that. issue #14 I believe. As far as I know, @Wilto is working on it!

On Jul 10, 2012, at 2:58 PM, Tomas wrote:

w.addEventListener( "resize", w.picturefill, false );

Sorry by commenting this here, but... do you have any particular reason why it's not throttled ?


Reply to this email directly or view it on GitHub:
#29 (comment)

@tomasdev

This comment has been minimized.

tomasdev commented Jul 12, 2012

Support for IE on mobile-first approach ?

matchMedia doesn't support it. (because it uses style[media])

I think I will be working on this list to create a cool pull request.

@scottjehl

This comment has been minimized.

Owner

scottjehl commented Jul 13, 2012

Hi @tomasdev,

I'm not sure I understand your question. Can you explain further?

Thanks

@tomasdev

This comment has been minimized.

tomasdev commented Jul 22, 2012

Let's say... On IE, matchMedia does nothing. That translates as "use the one without media query" in your script. Since noscript will not be read by browser, if you use a mobile first approach, you will endup looking the mobile image at IE (which is a desktop browser)

I wasn't able to figure out a cool solution for mobile-first picturefill with IE support.

@getdave

This comment has been minimized.

getdave commented Oct 4, 2012

For IE support you need a special fork of the matchmedia.js. You can find it at:

https://github.com/benschwarz/matchMedia.js/blob/IE7-8/matchMedia.js

Wilto added a commit to ResponsiveImagesCG/picture-refimp that referenced this issue Dec 19, 2012

@scottjehl

This comment has been minimized.

Owner

scottjehl commented Feb 28, 2014

Thanks for your ideas here. We're closing this out as Picturefill moves to support the new standard markup landing in browsers. Thread here: #125

@scottjehl scottjehl closed this Feb 28, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment