vega / polestar

Lightweight Tableau-style interface for visual analysis, built on Vega-lite.
http://vega.github.io/polestar
Other
367 stars 47 forks source link

Fix issue where display area does not take up full height #327

Closed kadamwhite closed 9 years ago

kadamwhite commented 9 years ago

See vega/vega-lite-ui#73; closes #322

This was introduced in dea6557d32c7733d220bd5dfca7bcc62f22b4dc8 while addressing an issue that prevented the bookmarks from closing: the display height issue arose due to the introduction of another layer of nesting that the CSS did not account for.

The same angular hierarchy can be achieved with less DOM nesting, so this commit addresses the overflow by removing unneeded intermediate div nodes.

Note that this did not end up being an issue with vega-lite-ui

This diff includes some indentation changes to main.html, so here is the diff with -w to ignore the whitespace:

diff --git a/src/app/main/main.html b/src/app/main/main.html
index 2fb43b2..80c0dd2 100644
--- a/src/app/main/main.html
+++ b/src/app/main/main.html
@@ -1,5 +1,4 @@
-<div ng-controller="MainCtrl">
-  <div class="flex-root vflex full-width full-height">
+<div ng-controller="MainCtrl" class="flex-root vflex full-width full-height">
   <div class="full-width no-shrink">
     <div class="card no-right-margin no-top-margin">
       <div class="right flex-root hflex">
@@ -76,7 +75,6 @@
       <vg-spec-editor></vg-spec-editor>
     </div>
   </div>
-  </div>
   <bookmark-list
     ng-if="Bookmarks.isSupported"
     active="bookmarksShown"
diff --git a/src/index.html b/src/index.html
index 185d0ea..4bf19ea 100644
--- a/src/index.html
+++ b/src/index.html
@@ -25,9 +25,7 @@
       <p class="browsehappy">You are using an <strong>outdated</strong> browser. Please <a href="http://browsehappy.com/">upgrade your browser</a> to improve your experience.</p>
     <![endif]-->

-    <div class="abs-100">
-      <ng-include src="'app/main/main.html'"></ng-include>
-    </div>
+    <div class="abs-100" ng-include="'app/main/main.html'"></div>

     <!-- build:js(src) scripts/vendor.js -->
     <!-- bower:js -->
kadamwhite commented 9 years ago

Note that bookmarks are broken currently, but that issue occurs in master as well so the regression is not caused by this commit

domoritz commented 9 years ago

What's broken about bookmarks? They work fine for me.