Page content's padding is not adjusted when fixed header elements changes its height due to a media query #5619

Closed
m90 opened this Issue Feb 8, 2013 · 4 comments

Projects

None yet

4 participants

@m90
m90 commented Feb 8, 2013

When using a fixed and persistent header that has several fixed height values for different screen states (i.e. media query) the pages' inline padding-top value (that will push the content down to fit underneath the header element) will not update accordingly.

See this demo fiddle and change the width of the viewport: the page's padding will be stuck with the value that it gets on initialization (min-height will update on $(window).resize() though).

@jaspermdegroot
Member

I think this is indeed a regression.
On resize updatePagePadding() should be called: https://github.com/jquery/jquery-mobile/blob/master/js/widgets/fixedToolbar.js#L119
I tested this local by replacing the call to updatePagePadding in that line by an alert and only got the alert after loading the page.
I tested throttledresize with a script in the head and then it did fire after resizing the window.
Not sure what's causing this problem.

@johnbender johnbender was assigned Feb 8, 2013
@scottjehl scottjehl was assigned Mar 28, 2013
@scottjehl scottjehl pushed a commit that referenced this issue Mar 29, 2013
Scott Jehl the first argument of updatePagePadding is expected to be a page, but…
… when it is called from the throttledResize binding, it ends up as an event object, and the padding was being set on that instead of the page. This adds a check to ensure the arg is a page before setting its padding. This is a fix for #5619
493229c
@scottjehl

Hey guys
The commit above works around this issue. For some reason, the throttledresize binding was passing its event object to the updatePagePadding function, though I think it was unintentional. As a result, the argument was defined and we were setting padding on that instead of the page element.

I worked around it by checking if the arg is an event or not before considering it a page. I'm unsure though if it'd be better to find a way to pass the page as an argument instead and avoid this check.

Either way, it's fixed now and we could land this and optimize later if desirable.

Let me know if it looks good to merge!

@scottjehl

Here's a JSbin with the fix running
http://jsbin.com/ozurob/1

@scottjehl scottjehl referenced this issue Mar 29, 2013
Merged

fix for 5619 #5826

@scottjehl scottjehl pushed a commit that referenced this issue Mar 29, 2013
Scott Jehl the first argument of updatePagePadding is expected to be a page, but…
… when it is called from the throttledResize binding, it ends up as an event object, and the padding was being set on that instead of the page. This adds a check to ensure the arg is a page before setting its padding. This is a fix for #5619 (cherry-picking from commit 493229c)
4f46cd4
@scottjehl

fixed.

@scottjehl scottjehl closed this Mar 29, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment