fix: 🐛 Smarter setting of bodyHeight based on overflow

When there are only a couple of rows, the scrollbar causes a scroll which is not needed. We check if the overflow amount is less than the size of scrollbar and then forcefully disable the scrolling.
frappe · Dec 22, 2018 · 302c1f8 · 302c1f8
1 parent d6aa52f
commit 302c1f8
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 31 deletions.
diff --git a/index.html b/index.html
@@ -10,7 +10,7 @@
 		body {
 			font-family: 'Tahoma';
 			font-weight: normal;
-			font-size: 14px;
+			font-size: 12px;
 		}
 	</style>
 	<link rel="stylesheet" href="./dist/frappe-datatable.css" />
@@ -28,11 +28,11 @@ <h1>Frappé DataTable</h1>
 		<input type="checkbox" id="dark-theme" />
 		<span>Dark Theme</span>
 	</label>
-	<section style="font-size: 12px; width: 60%; margin: 0 auto;">
+	<section style="width: 60%; margin: 0 auto;">
 
 	</section>
 
-	<section id="datatable2" style="width: 60%; font-size: 12px;">
+	<section id="datatable2" style="width: 60%; ">
 
 	</section>
 
@@ -153,7 +153,7 @@ <h1>Frappé DataTable</h1>
 					{ name: "Position" },
 					{ name: "Office" },
 					{ name: "Extn." },
-					{ name: "Start Date", format: (value) => value.split('/').reverse().join('/') },
+					{ name: "Start Date", /*format: (value) => value.split('/').reverse().join('/')*/ },
 					{ name: "Salary", format: value => formatMoney(value) }
 				];
 				data = [["Tiger Nixon", { content: "System Architect<br>New line", editable: false }, "Edinburgh", 5421, "2011/04/25", 320800], ["Garrett Winters", "Accountant", "Tokyo", 8422, "2011/07/25", 170750], ["Ashton Cox", "Junior Technical Author", "San Francisco", 1562, "2009/01/12", 86000], ["Cedric Kelly", "Senior Javascript Developer", "Edinburgh", 6224, "2012/03/29", 433060], ["Airi Satou", "Accountant", "Tokyo", 5407, "2008/11/28", 162700], ["Brielle Williamson", "Integration Specialist", "New York", 4804, "2012/12/02", 372000], ["Herrod Chandler", "Sales Assistant", "San Francisco", 9608, "2012/08/06", 137500], ["Rhona Davidson", "Integration Specialist", "Tokyo", 6200, "2010/10/14", 327900], ["Colleen Hurst", "Javascript Developer", "San Francisco", 2360, "2009/09/15", 205500], ["Sonya Frost", "Software Engineer", "Edinburgh", 1667, "2008/12/13", 103600], ["Jena Gaines", "Office Manager", "London", 3814, "2008/12/19", 90560], ["Quinn Flynn", "Support Lead", "Edinburgh", 9497, "2013/03/03", 342000], ["Charde Marshall", "Regional Director", "San Francisco", 6741, "2008/10/16", 470600], ["Haley Kennedy", "Senior Marketing Designer", "London", 3597, "2012/12/18", 313500], ["Tatyana Fitzpatrick", "Regional Director", "London", 1965, "2010/03/17", 385750], ["Michael Silva", "Marketing Designer", "London", 1581, "2012/11/27", 198500], ["Paul Byrd", "Chief Financial Officer (CFO)", "New York", 3059, "2010/06/09", 725000], ["Gloria Little", "Systems Administrator", "New York", 1721, "2009/04/10", 237500], ["Bradley Greer", "Software Engineer", "London", 2558, "2012/10/13", 132000], ["Dai Rios", "Personnel Lead", "Edinburgh", 2290, "2012/09/26", 217500], ["Jenette Caldwell", "Development Lead", "New York", 1937, "2011/09/03", 345000], ["Yuri Berry", "Chief Marketing Officer (CMO)", "New York", 6154, "2009/06/25", 675000], ["Caesar Vance", "Pre-Sales Support", "New York", 8330, "2011/12/12", 106450], ["Doris Wilder", "Sales Assistant", "Sidney", 3023, "2010/09/20", 85600], ["Angelica Ramos", "Chief Executive Officer (CEO)", "London", 5797, "2009/10/09", 1200000], ["Gavin Joyce", "Developer", "Edinburgh", 8822, "2010/12/22", 92575], ["Jennifer Chang", "Regional Director", "Singapore", 9239, "2010/11/14", 357650], ["Brenden Wagner", "Software Engineer", "San Francisco", 1314, "2011/06/07", 206850], ["Fiona Green", "Chief Operating Officer (COO)", "San Francisco", 2947, "2010/03/11", 850000], ["Shou Itou", "Regional Marketing", "Tokyo", 8899, "2011/08/14", 163000], ["Michelle House", "Integration Specialist", "Sidney", 2769, "2011/06/02", 95400], ["Suki Burks", "Developer", "London", 6832, "2009/10/22", 114500], ["Prescott Bartlett", "Technical Author", "London", 3606, "2011/05/07", 145000], ["Gavin Cortez", "Team Leader", "San Francisco", 2860, "2008/10/26", 235500], ["Martena Mccray", "Post-Sales support", "Edinburgh", 8240, "2011/03/09", 324050], ["Unity Butler", "Marketing Designer", "San Francisco", 5384, "2009/12/09", 85675], ["Howard Hatfield", "Office Manager", "San Francisco", 7031, "2008/12/16", 164500], ["Hope Fuentes", "Secretary", "San Francisco", 6318, "2010/02/12", 109850], ["Vivian Harrell", "Financial Controller", "San Francisco", 9422, "2009/02/14", 452500], ["Timothy Mooney", "Office Manager", "London", 7580, "2008/12/11", 136200], ["Jackson Bradshaw", "Director", "New York", 1042, "2008/09/26", 645750], ["Olivia Liang", "Support Engineer", "Singapore", 2120, "2011/02/03", 234500], ["Bruno Nash", "Software Engineer", "London", 6222, "2011/05/03", 163500], ["Sakura Yamamoto", "Support Engineer", "Tokyo", 9383, "2009/08/19", 139575], ["Thor Walton", "Developer", "New York", 8327, "2013/08/11", 98540], ["Finn Camacho", "Support Engineer", "San Francisco", 2927, "2009/07/07", 87500], ["Serge Baldwin", "Data Coordinator", "Singapore", 8352, "2012/04/09", 138575], ["Zenaida Frank", "Software Engineer", "New York", 7439, "2010/01/04", 125250], ["Zorita Serrano", "Software Engineer", "San Francisco", 4389, "2012/06/01", 115000], ["Jennifer Acosta", "Junior Javascript Developer", "Edinburgh", 3431, "2013/02/01", 75650], ["Cara Stevens", "Sales Assistant", "New York", 3990, "2011/12/06", 145600], ["Hermione Butler", "Regional Director", "London", 1016, "2011/03/21", 356250], ["Lael Greer", "Systems Administrator", "London", 6733, "2009/02/27", 103500], ["Jonas Alexander", "Developer", "San Francisco", 8196, "2010/07/14", 86500], ["Shad Decker", "Regional Director", "Edinburgh", 6373, "2008/11/13", 183000], ["Michael Bruce", "Javascript Developer", "Singapore", 5384, "2011/06/27", 183000], ["Donna Snider", "Customer Support", "New York", 4226, "2011/01/25", 112000]];
@@ -163,6 +163,8 @@ <h1>Frappé DataTable</h1>
 						data = data.concat(data);
 					}
 				}
+
+				// data = data.slice(1, 3)
 			}
 
 			function makeDatatable(treeView = false) {
@@ -178,6 +180,7 @@ <h1>Frappé DataTable</h1>
 					inlineFilters: true,
 					dynamicRowHeight: true,
 					treeView: treeView,
+					showTotalRow: true,
 					// filterRows(keyword, cells, colIndex) {
 					// 	return cells
 					// 		.filter(cell => cell.content.includes(keyword))

diff --git a/src/dom.js b/src/dom.js
@@ -180,7 +180,16 @@ $.scrollTop = function scrollTop(element, pixels) {
     });
 };
 
-$.scrollbarWidth = function scrollbarWidth() {
+$.scrollbarSize = function scrollbarSize() {
+    if (!$.scrollBarSizeValue) {
+        $.scrollBarSizeValue = getScrollBarSize();
+    }
+    return $.scrollBarSizeValue;
+};
+
+function getScrollBarSize() {
+    // assume scrollbar width and height would be the same
+
     // Create the measurement node
     const scrollDiv = document.createElement('div');
     $.style(scrollDiv, {
@@ -199,33 +208,16 @@ $.scrollbarWidth = function scrollbarWidth() {
     document.body.removeChild(scrollDiv);
 
     return scrollbarWidth;
-};
-
-$.scrollbarHeight = function scrollbarHeight() {
-    // Create the measurement node
-    const scrollDiv = document.createElement('div');
-    $.style(scrollDiv, {
-        width: '100px',
-        height: '100px',
-        overflow: 'scroll',
-        position: 'absolute',
-        top: '-9999px'
-    });
-    document.body.appendChild(scrollDiv);
-
-    // Get the scrollbar height
-    const scrollbarHeight = scrollDiv.offsetHeight - scrollDiv.clientHeight;
-
-    // Delete the DIV
-    document.body.removeChild(scrollDiv);
-
-    return scrollbarHeight;
-};
+}
 
 $.hasVerticalOverflow = function (element) {
     return element.scrollHeight > element.offsetHeight + 10;
 };
 
+$.hasHorizontalOverflow = function (element) {
+    return element.scrollWidth > element.offsetWidth + 10;
+};
+
 $.measureTextWidth = function (text) {
     const div = document.createElement('div');
     div.style.position = 'absolute';

diff --git a/src/style.js b/src/style.js
@@ -306,12 +306,33 @@ export default class Style {
 
         // when there are less rows than the container
         // adapt the container height
-        const height = $.getStyle(this.bodyScrollable, 'height');
+        let bodyHeight = $.getStyle(this.bodyScrollable, 'height');
         const scrollHeight = (this.bodyRenderer.hyperlist || {})._scrollHeight || Infinity;
-        const scrollbarHeight = $.scrollbarHeight();
-        if (scrollHeight < height) {
+        const hasHorizontalOverflow = $.hasHorizontalOverflow(this.bodyScrollable);
+
+        let height;
+
+        if (scrollHeight < bodyHeight) {
+            height = scrollHeight;
+
+            // account for scrollbar size when
+            // there is horizontal overflow
+            if (hasHorizontalOverflow) {
+                height += $.scrollbarSize();
+            }
+
+            $.style(this.bodyScrollable, {
+                height: height + 'px'
+            });
+        }
+
+        const verticalOverflow = this.bodyScrollable.scrollHeight - this.bodyScrollable.offsetHeight;
+        if (verticalOverflow < $.scrollbarSize()) {
+            // if verticalOverflow is less than scrollbar size
+            // then most likely scrollbar is causing the scroll
+            // which is not needed
             $.style(this.bodyScrollable, {
-                height: (scrollHeight + scrollbarHeight + 2) + 'px'
+                overflowY: 'hidden'
             });
         }