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

CSS Relative Units (vw, vh, vmin, vmax) - Re: Issue #572 #638

Merged
merged 5 commits into from
Aug 11, 2012

Conversation

glsee
Copy link
Contributor

@glsee glsee commented Jul 21, 2012

Kindly review. =)
Re: #572

@glsee glsee mentioned this pull request Jul 21, 2012
@ryanseddon
Copy link
Member

Would these need to be in 4 separate tests? I would imagine a browser vendor would implement them all at the same time.

They should probably be under one parent object like video or audio

Modernizr.cssviewportlengths = {
    vh: ...,
    vw: ...
    //etc
}

@glsee
Copy link
Contributor Author

glsee commented Jul 21, 2012

Chrome 20 implements vw, vh, vmin correctly but there's no support for vmax yet.

Would it be better if I put vw, vh and vmin under one parent object but vmax in a separate test?

@paulirish
Copy link
Member

seems okay as four separate tests. gzip would wipe out the byte duplication. kinda nice to keep things atomic.

getComputedStyle(elem, null) :
elem.currentStyle)["height"],10);

bool= !!(compStyle == height);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The !! Seems to be unnecessary

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, you're right. Removing them now.

@paulirish
Copy link
Member

lgtm. you, @ryanseddon ?

@KuraFire
Copy link
Member

@ryanseddon said:

Would these need to be in 4 separate tests? I would imagine a browser vendor would implement them all at the same time.

I think if Modernizr has taught me anything, it’s not to trust browser vendors for sensible things like that. ;-)

It looks good to me, merging.

KuraFire added a commit that referenced this pull request Aug 11, 2012
CSS Relative Units (vw, vh, vmin, vmax) - Re: Issue #572
@KuraFire KuraFire merged commit b18048b into Modernizr:master Aug 11, 2012
patrickkettner pushed a commit to patrickkettner/Modernizr that referenced this pull request Feb 22, 2015
CSS Relative Units (vw, vh, vmin, vmax) - Re: Issue Modernizr#572
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.

5 participants