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 the calculation notebook view #159

Merged
merged 4 commits into from
Nov 25, 2019
Merged

Fix the calculation notebook view #159

merged 4 commits into from
Nov 25, 2019

Conversation

bnmajor
Copy link
Contributor

@bnmajor bnmajor commented Oct 29, 2019

The view was previously limited to a height of 60px. The notebook height is now
determined by the available space.

Signed-off-by: Brianna Major [email protected]

The view was previously limited to a height of 60px. The notebook height is now
determined by the available space.

Signed-off-by: Brianna Major <[email protected]>
@bnmajor
Copy link
Contributor Author

bnmajor commented Oct 29, 2019

Addresses #150

const appStyles = theme => ({
iframe: {
width: '100%',
height: '-webkit-fill-available'
Copy link
Member

@cjh1 cjh1 Oct 29, 2019

Choose a reason for hiding this comment

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

Will this work on non webkit based browser? And do we care ? :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It will not, good point. I've updated the styling to calculate from the size of the notebook instead and this seems to work well in both Chrome and Firefox.

To ensure cross-browser support, calculate the height of the IFrame based on the
height of the notebook rather than using webkit styling.

Signed-off-by: Brianna Major <[email protected]>
@cjh1
Copy link
Member

cjh1 commented Nov 4, 2019

@bnmajor This works, but there a bit of delay, while the size is calculated. So it looks like this for a second or so:

Screenshot from 2019-11-04 10-10-26

Is there a way we could do it with CSS across browsers or hidden the component util the calculation is done.

Use the notebook aspect ratio to determine padding for the iFrame container. This
keeps the proportions of the JupyterLab Notebook, avoids the delay caused by
calculating the height, and seems to work smoothly across browsers.

Signed-off-by: Brianna Major <[email protected]>
container: {
overflow: 'hidden',
position: 'relative',
paddingTop: '72%'
Copy link
Member

Choose a reason for hiding this comment

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

Let give ourselves a little more height and increase this to 100%

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@bnmajor bnmajor merged commit 269f220 into master Nov 25, 2019
@bnmajor bnmajor deleted the notebook-view branch November 25, 2019 15:17
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.

2 participants