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

Calculate height dynamically using jquery for scrollable sqllab #1611

Merged
merged 3 commits into from
Nov 22, 2016

Conversation

vera-liu
Copy link
Contributor

Before:
Previously we used margin_bottom and height:95% in css for scrollable components

After:

  • calculate height based on window size, navbar height and tab height
  • event handler for window resize

screen shot 2016-11-15 at 4 30 05 pm

needs-review @mistercrunch

@mistercrunch
Copy link
Member

mistercrunch commented Nov 16, 2016

This is flawed again:

  • Tab height should be dynamic, not hard coded as many tabs overflow into a second row of tabs
  • Remaining height only needs to be calculated in one place (SqlEditor)
  • Should take into account the staging warning
  • Should take into account the Alerts

@vera-liu
Copy link
Contributor Author

vera-liu commented Nov 16, 2016

Pushed a new commit:

  • parent is mounted after child mounting in react, so to get the height of Alerts and Tab size, I have to calculate it in App.jsx
  • it seems the only suitable place to calculate height is in componentDidMount, after everything is rendered, but if we setState in componentDidMount, it will trigger another re-rendering, so it will render two times every time

@mistercrunch

@vera-liu vera-liu force-pushed the vliu_solve_sqllab_height_problem branch 3 times, most recently from 3c0a1da to a0505e0 Compare November 16, 2016 05:00
@mistercrunch
Copy link
Member

LGTM

@vera-liu vera-liu force-pushed the vliu_solve_sqllab_height_problem branch from a0505e0 to 0c20db5 Compare November 22, 2016 19:18
@vera-liu vera-liu force-pushed the vliu_solve_sqllab_height_problem branch from 0c20db5 to c39c399 Compare November 22, 2016 19:36
@vera-liu vera-liu merged commit db1ed2a into apache:master Nov 22, 2016
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.14.0 labels Feb 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.14.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants