- RIP smooth scrolling and associated hacks

- attempt to make Headlines.move() / Article.cdmMoveToId() behave a bit more intuitively

4 files changed, 54 insertions, 67 deletions

diff --git a/js/AppBase.js b/js/AppBase.js
index c4882267d..4aa500e5b 100644
--- a/js/AppBase.js
+++ b/js/AppBase.js
@@ -8,29 +8,17 @@ define(["dojo/_base/declare"], function (declare) {
 		hotkey_prefix_pressed: false,
 		hotkey_prefix_timeout: 0,
 		Scrollable: {
-			scrollByPages: function (elem, page_offset, event) {
+			scrollByPages: function (elem, page_offset) {
 				if (!elem) return;
 
 				/* keep a line or so from the previous page  */
 				const offset = (elem.offsetHeight - (page_offset > 0 ? 50 : -50)) * page_offset;
 
-				this.scroll(elem, offset, event);
+				this.scroll(elem, offset);
 			},
-			scroll: function(elem, offset, event) {
+			scroll: function(elem, offset) {
 				if (!elem) return;
 
-				if (event && event.repeat) {
-					elem.addClassName("forbid-smooth-scroll");
-					window.clearTimeout(this._scroll_reset_timeout);
-
-					this._scroll_reset_timeout = window.setTimeout(() => {
-						if (elem) elem.removeClassName("forbid-smooth-scroll");
-					}, 250)
-
-				} else {
-					elem.removeClassName("forbid-smooth-scroll");
-				}
-
 				elem.scrollTop += offset;
 			},
 			isChildVisible: function(elem, ctr) {
@@ -45,6 +33,12 @@ define(["dojo/_base/declare"], function (declare) {
 				return etop >= ctop && ebottom <= cbottom ||
 					etop < ctop && ebottom > ctop || ebottom > cbottom && etop < cbottom;
 			},
+			fitsInContainer: function (elem, ctr) {
+				if (!elem) return;
+
+				return elem.offsetTop + elem.offsetHeight <= ctr.scrollTop + ctr.offsetHeight &&
+					elem.offsetTop >= ctr.scrollTop;
+			}
 		},
 		constructor: function() {
 			window.onerror = this.Error.onWindowError;
diff --git a/js/Article.js b/js/Article.js
index b0021f47b..3056dd666 100644
--- a/js/Article.js
+++ b/js/Article.js
@@ -196,11 +196,11 @@ define(["dojo/_base/declare"], function (declare) {
 				row.querySelector(".content-inner").innerHTML = "&nbsp;";
 			}
 		},
-		view: function (id, noexpand) {
+		view: function (id, no_expand) {
 			this.setActive(id);
 			Headlines.scrollToArticleId(id);
 
-			if (!noexpand) {
+			if (!no_expand) {
 				const hl = Headlines.objectById(id);
 
 				if (hl) {
@@ -294,32 +294,15 @@ define(["dojo/_base/declare"], function (declare) {
 		cdmMoveToId: function (id, params) {
 			params = params || {};
 
-			const force = params.force || true;
-			const event = params.event || null;
-			const noscroll = params.noscroll || false;
+			const force_to_top = params.force_to_top || false;
 
 			const ctr = $("headlines-frame");
-			const e = $("RROW-" + id);
-			const is_expanded = App.getInitParam("cdm_expanded");
+			const row = $("RROW-" + id);
 
-			if (!e || !ctr) return;
+			if (!row || !ctr) return;
 
-			if (force || is_expanded || e.offsetTop + e.offsetHeight > (ctr.scrollTop + ctr.offsetHeight) ||
-				e.offsetTop < ctr.scrollTop) {
-
-				if (noscroll || event && event.repeat || !is_expanded) {
-					ctr.addClassName("forbid-smooth-scroll");
-					window.clearTimeout(this._scroll_reset_timeout);
-
-					this._scroll_reset_timeout = window.setTimeout(() => {
-						if (ctr) ctr.removeClassName("forbid-smooth-scroll");
-					}, 250)
-
-				} else {
-					ctr.removeClassName("forbid-smooth-scroll");
-				}
-
-				ctr.scrollTop = e.offsetTop;
+			if (force_to_top || !App.Scrollable.fitsInContainer(row, ctr)) {
+				ctr.scrollTop = row.offsetTop;
 			}
 		},
 		setActive: function (id) {
@@ -351,11 +334,11 @@ define(["dojo/_base/declare"], function (declare) {
 			else
 				return 0;
 		},
-		scrollByPages: function (page_offset, event) {
-			App.Scrollable.scrollByPages($("content-insert"), page_offset, event);
+		scrollByPages: function (page_offset) {
+			App.Scrollable.scrollByPages($("content-insert"), page_offset);
 		},
-		scroll: function (offset, event) {
-			App.Scrollable.scroll($("content-insert"), offset, event);
+		scroll: function (offset) {
+			App.Scrollable.scroll($("content-insert"), offset);
 		},
 		mouseIn: function (id) {
 			this.post_under_pointer = id;
diff --git a/js/Headlines.js b/js/Headlines.js
index 0e929fe30..c0c03d724 100755
--- a/js/Headlines.js
+++ b/js/Headlines.js
@@ -204,14 +204,23 @@ define(["dojo/_base/declare"], function (declare) {
 					} else if (Article.getActive() != id) {
 
 						Headlines.select('none');
+
+						const scroll_position_A = $("RROW-" + id).offsetTop - $("headlines-frame").scrollTop;
+
 						Article.setActive(id);
 
 						if (App.getInitParam("cdm_expanded")) {
+
 							if (!in_body)
 								Article.openInNewWindow(id);
 
 							Headlines.toggleUnread(id, 0);
 						} else {
+							const scroll_position_B = $("RROW-" + id).offsetTop - $("headlines-frame").scrollTop;
+
+							// this would only work if there's enough space
+							$("headlines-frame").scrollTop -= scroll_position_A-scroll_position_B;
+
 							Article.cdmMoveToId(id);
 						}
 
@@ -794,10 +803,9 @@ define(["dojo/_base/declare"], function (declare) {
 		move: function (mode, params) {
 			params = params || {};
 
-			const noscroll = params.noscroll || false;
-			const noexpand = params.noexpand || false;
+			const no_expand = params.no_expand || false;
 			const force_previous = params.force_previous || false;
-			const event = params.event;
+			const force_to_top = params.force_to_top || false;
 
 			let prev_id = false;
 			let next_id = false;
@@ -835,10 +843,10 @@ define(["dojo/_base/declare"], function (declare) {
 					if (App.isCombinedMode()) {
 						window.requestAnimationFrame(() => {
 							Article.setActive(next_id);
-							Article.cdmMoveToId(next_id, {event: event, noscroll: noscroll});
+							Article.cdmMoveToId(next_id, {force_to_top: force_to_top});
 						});
 					} else {
-						Article.view(next_id, noexpand);
+						Article.view(next_id, no_expand);
 					}
 				}
 			} else if (mode === "prev") {
@@ -847,18 +855,20 @@ define(["dojo/_base/declare"], function (declare) {
 						window.requestAnimationFrame(() => {
 							const row = $("RROW-" + current_id);
 							const ctr = $("headlines-frame");
-							const delta_px = Math.max(row.offsetTop, ctr.scrollTop) - Math.min(row.offsetTop, ctr.scrollTop);
+							const delta_px = Math.round(row.offsetTop) - Math.round(ctr.scrollTop);
+
+							console.log('moving back, delta_px', delta_px);
 
-							if (!force_previous && row && delta_px > 16) {
+							if (!force_previous && row && delta_px < -8) {
 								Article.setActive(current_id);
-								Article.cdmMoveToId(current_id, {force: noscroll, event: event});
+								Article.cdmMoveToId(current_id, {force_to_top: force_to_top});
 							} else if (prev_id) {
 								Article.setActive(prev_id);
-								Article.cdmMoveToId(prev_id, {force: noscroll, event: event, noscroll: noscroll});
+								Article.cdmMoveToId(prev_id, {force_to_top: force_to_top});
 							}
 						});
 					} else if (prev_id) {
-						Article.view(prev_id, noexpand);
+						Article.view(prev_id, no_expand);
 					}
 				}
 			}
@@ -1343,11 +1353,11 @@ define(["dojo/_base/declare"], function (declare) {
 
 			}
 		},
-		scrollByPages: function (page_offset, event) {
-			App.Scrollable.scrollByPages($("headlines-frame"), page_offset, event);
+		scrollByPages: function (page_offset) {
+			App.Scrollable.scrollByPages($("headlines-frame"), page_offset);
 		},
-		scroll: function (offset, event) {
-			App.Scrollable.scroll($("headlines-frame"), offset, event);
+		scroll: function (offset) {
+			App.Scrollable.scroll($("headlines-frame"), offset);
 		},
 		initHeadlinesMenu: function () {
 			if (!dijit.byId("headlinesMenu")) {
diff --git a/js/tt-rss.js b/js/tt-rss.js
index 595766f4e..d8818420c 100644
--- a/js/tt-rss.js
+++ b/js/tt-rss.js
@@ -288,25 +288,25 @@ require(["dojo/_base/kernel",
 						if (App.isCombinedMode())
 							Headlines.scroll(Headlines.line_scroll_offset, event);
 						else
-							Headlines.move('next', {event: event});
+							Headlines.move('next');
 					};
 					this.hotkey_actions["prev_article_or_scroll"] = function (event) {
 						if (App.isCombinedMode())
 							Headlines.scroll(-Headlines.line_scroll_offset, event);
 						else
-							Headlines.move('prev', {event: event});
+							Headlines.move('prev');
 					};
-					this.hotkey_actions["next_article_noscroll"] = function (event) {
-						Headlines.move('next', {noscroll: true, event: event});
+					this.hotkey_actions["next_article_noscroll"] = function () {
+						Headlines.move('next');
 					};
-					this.hotkey_actions["prev_article_noscroll"] = function (event) {
-						Headlines.move('prev', {noscroll: true, event: event});
+					this.hotkey_actions["prev_article_noscroll"] = function () {
+						Headlines.move('prev');
 					};
-					this.hotkey_actions["next_article_noexpand"] = function (event) {
-						Headlines.move('next', {noscroll: true, noexpand: true, event: event});
+					this.hotkey_actions["next_article_noexpand"] = function () {
+						Headlines.move('next', {no_expand: true});
 					};
-					this.hotkey_actions["prev_article_noexpand"] = function (event) {
-						Headlines.move('prev', {noscroll: true, noexpand: true, event: event});
+					this.hotkey_actions["prev_article_noexpand"] = function () {
+						Headlines.move('prev', {no_expand: true});
 					};
 					this.hotkey_actions["search_dialog"] = function () {
 						Feeds.search();