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

Table Sizing #180

Merged
merged 4 commits into from
Nov 8, 2016
Merged

Table Sizing #180

merged 4 commits into from
Nov 8, 2016

Conversation

curran
Copy link
Contributor

@curran curran commented Nov 7, 2016

Will close #101

@curran
Copy link
Contributor Author

curran commented Nov 7, 2016

Here's the current solution:

image

This actually uncovers some upstream errors with our graph sizing in general. Namely:

  • There is always a 10px padding applied. This should not be the case, as the visualization should be able to completely fill up the rectangle of its parent grid item.
  • There seems to be an error in the width calculation, which gives 10px extra on the right side.

@curran
Copy link
Contributor Author

curran commented Nov 7, 2016

Here's what it looks like after eliminating the padding that was applied to all visualizations:

image

@curran
Copy link
Contributor Author

curran commented Nov 7, 2016

Here's what the final solution looks like:

image

@t00f @ronakmshah This is ready for review and merge.

@curran
Copy link
Contributor Author

curran commented Nov 7, 2016

Note that this does actually impact all the graphs, by eliminating the 10px padding that was previously applied to all of them. If we want more breathing room inside the graphs, I suggest we do it by modifying their margins.

@curran curran mentioned this pull request Nov 7, 2016
40 tasks
@ronakmshah
Copy link
Contributor

Thanks @curran for this work. Looks good. Nit:
![screen shot 2016-11-07 at 3 22 38 pm]Ihttps://cloud.githubusercontent.com/assets/1153786/20080347/1e01cbca-a4fe-11e6-94a0-88fc8121de97.png)
screen shot 2016-11-07 at 3 22 18 pm

We need to implement fit to size to the table. Once done, this is good for merging.

@curran
Copy link
Contributor Author

curran commented Nov 8, 2016

@ronakmshah Thanks for testing, I will try to fix that.

@curran
Copy link
Contributor Author

curran commented Nov 8, 2016

Fixed:

image

@ronakmshah
Copy link
Contributor

Looks good.

@ronakmshah ronakmshah merged commit b95ab44 into master Nov 8, 2016
@ronakmshah ronakmshah deleted the table-sizing branch July 6, 2017 16:57
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.

Table sizing
2 participants