Merge pull request #158 from ppaskaris-d2l/fix-sticky-bootans

Fix stickies in Chrome/FF and remove bonus margins in IE/Edge

.travis.yml

@@ -23,7 +23,7 @@ before_install:
 - export JAVA_HOME=/usr/lib/jvm/java-8-oracle
 - export PATH=$JAVA_HOME/bin:$PATH
 script:
-- concurrently -p name -n lint:js,lint:wc,wct "npm run lint:js" "npm run lint:wc" "wct --skip-plugin local" && npm run test:galen:sauce
+- concurrently -p name -n lint:js,lint:wc,polymer:test "npm run lint:js" "npm run lint:wc" "polymer test --skip-plugin local" && npm run test:galen:sauce
 env:
   global:
     SAUCE_USERNAME: Desire2Learn

bower.json

@@ -26,7 +26,7 @@
     "d2l-icons": "^4.5.1",
     "iron-resizable-behavior": "^2.0.1",
     "polymer": "1 - 2",
-    "Stickyfill": "^1.1.4"
+    "Stickyfill": "^2.0.3"
   },
   "devDependencies": {
     "d2l-demo-template": "^1.0.3",

d2l-scroll-wrapper.html

@@ -99,8 +99,19 @@
 				};
 
 				--d2l-scroll-wrapper-sticky: {
+					height: 0;
 					/* height of button (40) + distance of button from top (10) + desired spacing (10) */
 					margin-bottom: 60px;
+					margin-top: -60px;
+					overflow: visible;
+				};
+
+				--d2l-scroll-wrapper-sticky-hidden: {
+					display: none;
+				};
+
+				--d2l-scroll-wrapper-sticky-visible: {
+					display: block;
 				};
 
 				--d2l-scroll-wrapper-action-hidden: {
@@ -157,19 +168,21 @@
 
 			.action {
 				@apply --d2l-scroll-wrapper-action;
+				@apply --d2l-scroll-wrapper-action-visible;
 			}
+
 			.sticky {
-				@apply --d2l-scroll-wrapper-action-hidden;
 				@apply --d2l-scroll-wrapper-sticky;
+				@apply --d2l-scroll-wrapper-sticky-hidden;
 			}
 
-			:host([is-rtl]) .left .action,
-			.right .action {
+			:host([is-rtl]) .left.action,
+			.right.action {
 				@apply --d2l-scroll-wrapper-action-end;
 			}
 
-			:host([is-rtl]) .right .action,
-			.left .action {
+			:host([is-rtl]) .right.action,
+			.left.action {
 				@apply --d2l-scroll-wrapper-action-start;
 			}
 
@@ -192,27 +205,25 @@
 			}
 
 			:host([show-actions][h-scrollbar]) .sticky {
-				@apply --d2l-scroll-wrapper-action-visible;
+				@apply --d2l-scroll-wrapper-sticky-visible;
 			}
 
 			/* Hide the start/end buttons depending on the state */
-			:host([show-actions][scrollbar-right]) .right,
-			:host([show-actions][scrollbar-left]) .left {
+			:host([show-actions][scrollbar-right]) .right.action,
+			:host([show-actions][scrollbar-left]) .left.action {
 				@apply --d2l-scroll-wrapper-action-hidden;
 			}
 		</style>
-		<d2l-sticky-element class="left sticky" disabled="[[scrollbarLeft]]">
+		<d2l-sticky-element class="sticky" disabled="[[_stickyIsDisabled]]">
 			<d2l-icon-button
-				class="action"
+				class="left action"
 				icon="[[startIcon]]"
 				on-tap="handleTapLeft"
 				tabindex="-1"
 				aria-hidden="true"
 			></d2l-icon-button>
-		</d2l-sticky-element>
-		<d2l-sticky-element class="right sticky" disabled="[[scrollbarRight]]">
 			<d2l-icon-button
-				class="action"
+				class="right action"
 				icon="[[endIcon]]"
 				on-tap="handleTapRight"
 				tabindex="-1"
@@ -361,6 +372,11 @@
 					showActions: {
 						type: Boolean,
 						reflectToAttribute: true
+					},
+
+					_stickyIsDisabled: {
+						type: Boolean,
+						computed: '_computeStickyIsDisabled(scrollbarLeft, scrollbarRight)'
 					}
 				},
 
@@ -483,6 +499,10 @@
 						this._setScrollbarLeft(this.$.wrapper.scrollLeft === 0);
 						this._setScrollbarRight(lowerScrollValue <= 0);
 					}
+				},
+
+				_computeStickyIsDisabled: function(scrollbarLeft, scrollbarRight) {
+					return Boolean(scrollbarLeft) && Boolean(scrollbarRight);
 				}
 			});
 		})();

d2l-sticky-element.html

@@ -28,6 +28,24 @@
 					observer: '_disabledChanged'
 				}
 			},
+			detached: function() {
+				try {
+					this._ensureStickyRemoved();
+				} catch (e) {
+					return;
+				}
+			},
+			_ensureStickyAdded: function() {
+				if (this._sticky) return;
+				this._sticky = Stickyfill.addOne(this);
+			},
+			_ensureStickyRemoved: function() {
+				if (!this._sticky) return;
+				// If remove fails, still unset _sticky.
+				var tmp = this._sticky;
+				this._sticky = null;
+				tmp.remove();
+			},
 			_updateSticky: function() {
 				/**
 				 * Stickyfill requires the component to be attached to the DOM
@@ -40,22 +58,17 @@
 				var sticky = !this.disabled;
 				try {
 					if (sticky) {
-						Stickyfill.add(this);
-						this._added = true;
-					} else if (this._added) {
-						Stickyfill.remove(this);
-						this._added = false;
+						this._ensureStickyAdded();
 					} else {
-						return;
+						this._ensureStickyRemoved();
 					}
 				} catch (e) {
 					return;
 				}
-				Stickyfill.rebuild();
 			},
 			_disabledChanged: function() {
 				// Defer the getComputedStyle() calls until after the page has rendered
-				Polymer.RenderStatus.afterNextRender(this, this._updateSticky.bind(this));
+				Polymer.RenderStatus.afterNextRender(this, this._updateSticky);
 			}
 		});
 	</script>

demo/index.html

@@ -21,6 +21,13 @@
                 .screenshots {
                   display: inline-block;
                 }
+
+				/* allow the demo to show the scroll actions clamping */
+				.scroll-action-spacer {
+					font-style: italic;
+					padding-bottom: 600px;
+					padding-top: 600px;
+				}
         	</style>
         </custom-style>
     </head>
@@ -1058,6 +1065,10 @@
     			</tr>
     			</tbody>
     		</table></d2l-table-wrapper>
+			<div class="scroll-action-spacer">
+				This space is intentionally consumed to demonstrate the scroll
+				chevrons clamping at the bottom of the table.
+			</div>
     	</div>
 		</d2l-demo-template>
     </body>

package.json

@@ -7,7 +7,7 @@
     "postinstall": "polymer install --variants",
     "predump": "npm run -s clean",
     "dump": "npm run -s dump:galen:local",
-    "test": "concurrently -p name -n lint:js,lint:wc,wct \"npm run lint:js\" \"npm run lint:wc\" \"npm run test:galen:local\" \"wct --skip-plugin sauce\"",
+    "test": "concurrently -p name -n lint:js,lint:wc,polymer:test \"npm run lint:js\" \"npm run lint:wc\" \"npm run test:galen:local\" \"polymer test --skip-plugin sauce\"",
     "lint": "npm run lint:js && npm run lint:wc",
     "lint:js": "eslint --ext html .",
     "lint:wc": "polymer lint --input *.html",

test/acceptance/table.gspec

@@ -42,14 +42,14 @@
 
 	wide                  .screenshot-wide d2l-table-wrapper
 		scroll-wrapper    ::d2l-shadow d2l-scroll-wrapper
-			left-action   ::d2l-shadow .left > d2l-icon-button
-			right-action  ::d2l-shadow .right > d2l-icon-button
+			left-action   ::d2l-shadow d2l-icon-button.left
+			right-action  ::d2l-shadow d2l-icon-button.right
 		tr-*              tr
 
 	d2l-wide              .screenshot-d2l-wide d2l-table
 		scroll-wrapper    ::d2l-shadow d2l-scroll-wrapper
-			left-action   ::d2l-shadow .left > d2l-icon-button
-			right-action  ::d2l-shadow .right > d2l-icon-button
+			left-action   ::d2l-shadow d2l-icon-button.left
+			right-action  ::d2l-shadow d2l-icon-button.right
 		tr-*              d2l-tr
 
 	no-column-border      .screenshot-no-column-border
@@ -108,13 +108,13 @@
 
 		wide                  .screenshot-wide d2l-table-wrapper
 			scroll-wrapper    d2l-scroll-wrapper
-				left-action   .left > d2l-icon-button
-				right-action  .right > d2l-icon-button
+				left-action   d2l-icon-button.left
+				right-action  d2l-icon-button.right
 
 		d2l-wide              .screenshot-d2l-wide d2l-table
 			scroll-wrapper    d2l-scroll-wrapper
-				left-action   .left > d2l-icon-button
-				right-action  .right > d2l-icon-button
+				left-action   d2l-icon-button.left
+				right-action  d2l-icon-button.right
 
 @groups
 	baseImageCompare small.head,small.body,small.foot,d2l-small.head,d2l-small.body,d2l-small.foot,simple.head,simple.body,simple.foot,no-column-border.head,no-column-border.body,tr-last-of-type.head,tr-last-of-type.body,notbody,row-header-with-thead.head,row-header-with-thead.body,row-header-without-thead.body,dynamic.head,dynamic.body,d2l-dynamic.head,d2l-dynamic.body
@@ -164,13 +164,15 @@
 					image file dumps/${path}/objects/${tr}.png, error 40px
 
 	@on rtl
-		wide.scroll-wrapper.right-action:
-			inside partly wide 10px top, -15px left
-			image file dumps/${path}/objects/wide.scroll-wrapper.right-action.png, error 20px
-
-		d2l-wide.scroll-wrapper.right-action:
-			inside partly d2l-wide 10px top, -15px left
-			image file dumps/${path}/objects/d2l-wide.scroll-wrapper.right-action.png, error 20px
+		# There is a bug on Edge's sticky implementation. So skip the rtl test on edge
+		@on sticky
+			wide.scroll-wrapper.right-action:
+				inside partly wide 10px top, -15px left
+				image file dumps/${path}/objects/wide.scroll-wrapper.right-action.png, error 20px
+
+			d2l-wide.scroll-wrapper.right-action:
+				inside partly d2l-wide 10px top, -15px left
+				image file dumps/${path}/objects/d2l-wide.scroll-wrapper.right-action.png, error 20px
 	@on ltr
 		wide.scroll-wrapper.right-action:
 			inside partly wide 10px top, 275px left

test/d2l-scroll-wrapper.html

@@ -264,12 +264,12 @@
 						});
 
 						it('should have only one visible action button pointing right', function() {
-							var actions = Polymer.dom(this.scrollWrapper.root).querySelectorAll('.sticky');
+							var actions = Polymer.dom(this.scrollWrapper.root).querySelectorAll('.action');
 							actions = [].filter.call(actions, function(action) {
 								return getComputedStyle(action).display !== 'none';
 							});
 							expect(actions).to.have.lengthOf(1);
-							var style = getComputedStyle(Polymer.dom(actions[0]).firstChild);
+							var style = getComputedStyle(actions[0]);
 							expect(style.right).to.equal('-15px');
 						});
 					});
@@ -306,12 +306,12 @@
 						});
 
 						it('should have only one visible action button pointing left', function() {
-							var actions = Polymer.dom(this.scrollWrapper.root).querySelectorAll('.sticky');
+							var actions = Polymer.dom(this.scrollWrapper.root).querySelectorAll('.action');
 							actions = [].filter.call(actions, function(action) {
 								return getComputedStyle(action).display !== 'none';
 							});
 							expect(actions).to.have.lengthOf(1);
-							var style = getComputedStyle(Polymer.dom(actions[0]).firstChild);
+							var style = getComputedStyle(actions[0]);
 							expect(style.left).to.equal('-15px');
 						});
 					});