Skip to content
This repository has been archived by the owner on Aug 5, 2020. It is now read-only.

stop viewport being set at window.innerHeight #69

Merged
merged 6 commits into from
Mar 14, 2016

Conversation

ericawright
Copy link

Status: Ready for Review
Owner: @ericawright
Reviewers: @mikenikles @fractaltheory @jansepar @marlowpayne @stewartyu

Changes

  • on Chrome when scrolled down the page the viewport was staying at the top of the page and remained the size of the window.
  • remove the body height style, as it was being set to window size
  • stop viewport height being set as inner height
  • used css to add safari-specific code:
    • chrome browser needs min-height set at 100% -- otherwise we have orientation issues
    • safari browser needs height: 100% -- otherwise orientation and scroll issues

Todos:

  • Engineer +1

Issues:

#66
#49

Feedback:

none so far

How to Test

@marlowpayne
Copy link

Very cool stuff @ericawright ! Just a note that when you touch the src code for Pikabu you'll want to re-build the dist/ files with grunt build and commit them too.

@ericawright
Copy link
Author

@marlowpayne thanks, the dist/ files slipped my mind

@mikenikles
Copy link

Nice fix, @ericawright! 👍

@ericawright
Copy link
Author

ready to go again after noticing an orientation bug

@fractaltheory
Copy link

LGTM! 👍

There's a test failing but it's failing on develop too so it doesn't block merge -- if you have time @ericawright it'd be awesome to get that fixed too.

screen shot 2016-03-04 at 4 04 28 pm

@ericawright
Copy link
Author

@fractaltheory yup, that test is fixed in #76

@fractaltheory
Copy link

💯 !

ericawright added a commit that referenced this pull request Mar 14, 2016
stop viewport being set at window.innerHeight
@ericawright ericawright merged commit 5617915 into develop Mar 14, 2016
@ericawright ericawright deleted the fix/chrome-scroll-bug branch March 14, 2016 16:22
@jeffkamo jeffkamo mentioned this pull request Mar 29, 2016
1 task
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants