Skip to content
This repository has been archived by the owner on Sep 6, 2022. It is now read-only.

Fixing Regex to work with multiple Media Query rules #487

Closed
wants to merge 1 commit into from

Conversation

paulomunoz
Copy link

Fixing Regex to work with multiple Media Query rules, like: "(min-height: 900px) and (min-width: 992px) 150px"
2015-04-16_1845
2015-04-16_1845_001
2015-04-16_1843

…in-height: 900px) and (min-width: 992px) 150px, 60px"
@paulomunoz
Copy link
Author

I found this limitation while trying to execute the following code:

<img src="assets/img/ef-logo-200.png" alt="EF Education First" 
                sizes="(min-height: 900px) and (min-width: 992px) 150px,(min-width:768px) 100px, 60px"
                srcset="assets/img/ef-logo-60.png 60w,
                      assets/img/ef-logo-100.png 100w,
                      assets/img/ef-logo-120.png 120w,
                      assets/img/ef-logo-200.png 200w" />

I fixed the regular expression but I wasn't sure which branch I should request a pull to, did it for Master but let me know if I should do it for a different branch.

On Chrome (which has HTMLPictureElement) it works fine, but on Firefox for example, the rule (min-height: 900px) and (min-width: 992px) 150px was never working. Now it seems to be fine, I tried to test some other variations and it all seems to be ok now. Please let me know if you find any problem.

Thanks,
Paulo

@albell
Copy link
Collaborator

albell commented Apr 16, 2015

Yeah, much needed. But... my parser rewrite in the PF3 proposal handles all these cases correctly, and more, and much closer to spec. So +1 but we ought to think about when we're going to shift energy towards PF3 proposal.

@Wilto
Copy link
Collaborator

Wilto commented Apr 16, 2015

+1 from me as well.

@baloneysandwiches: you, me, @aFarkas, and the rest of the team should block out a little time next week to talk about where things stand on the PF3 proposal. Once you guys are feeling pretty good about a 3.0.0, I want to get some dedicated beta testing volunteers. This one is gonna be too big to just chuck out there like “okay, so, everybody test this when you get a sec.”

@aFarkas
Copy link
Collaborator

aFarkas commented Apr 16, 2015

We already have something like this in our 2.4 branch. Can you check whether this solves your problem. See: https://github.com/scottjehl/picturefill/blob/2.4/src/picturefill.js#L183

@mike-engel
Copy link
Collaborator

@paulomunoz If you get some time, can you checkout our 2.4 branch, or 3.0-beta to see if the updates are what you're looking for?

@aFarkas
Copy link
Collaborator

aFarkas commented May 29, 2015

I think we can close this.

@mike-engel mike-engel closed this Jun 1, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants