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

Fix Safari terminal window height bug #1988

Merged
merged 2 commits into from
Nov 7, 2016
Merged

Conversation

jpellizzari
Copy link
Contributor

@jpellizzari jpellizzari commented Nov 4, 2016

Fix for #1986
Fixes the issue where the terminal in Safari isn't the right size.
Before:
screen shot 2016-11-04 at 1 09 57 pm
After:
screen shot 2016-11-04 at 1 10 05 pm

@jpellizzari
Copy link
Contributor Author

@bowenli could you review?

@jpellizzari
Copy link
Contributor Author

@foot Could you take a look at this?

@foot foot assigned foot and unassigned bowenli Nov 7, 2016
@foot
Copy link
Contributor

foot commented Nov 7, 2016

Its kind of a fun effect where it slides out compact and then grows vertically.

Nice catch on making the terminal padding "border" black to match the term.

foot added 2 commits November 7, 2016 10:22
Safari wasn't supporting a "badly specified"[0] layout that Chrome was
supporting.

[0] Height 100% inside of a container that didn't have a height
specified.
@foot foot force-pushed the 1986-safari-terminal branch from 45e3296 to 127ab41 Compare November 7, 2016 09:38
@foot
Copy link
Contributor

foot commented Nov 7, 2016

I changed the solution a bit and instead fixed up the CSS layout so that height was propagated down the DOM tree in Safari.

All the nesting for the Terminal here is a bit icky, mix of absolute+top0+bottom0 and height 100%s but works for now.

@foot foot merged commit 942ccc8 into release-1.0 Nov 7, 2016
@foot foot deleted the 1986-safari-terminal branch November 7, 2016 11:58
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.

3 participants