Since the last commit 8dc6b48ebd, the hotkey action prev_article_noscroll has stopped working reliably: in some feeds, there is no visual response whatsoever.
I reverted to the commit before and it started working again in all situations.
I am using the action in combined mode (not expanded), trying to open the previous article directly, also when the current article is opened, using the following code:
var id = Headlines.getSelected();
App.hotkey_actions["prev_article_noscroll"]();
if (id && id == Headlines.getSelected()) // Still on the same article
App.hotkey_actions["prev_article_noscroll"]();
those actions supposed to be called as keyboard shortcuts, not plugin (?) code.
some keyboard actions may return when action has not completed yet, for example headlines buffer is still scrolling up. you’re calling them as synchronous code. this is a really bad idea.
Headlines.getSelected() returns an array of selected headlines, of which there could be more than one.
btw if what you’re trying to accomplish is always moving to previous article regardless of current position, i think a better idea would be allowing this via Headlines.move() and/or adding a separate hotkey action you could bind.
Yes, you’re right, and the currently released version of that plugin just uses Headlines.move(key, {noscroll: true});. I was thinking that I could perhaps directly call the hotkey action as an extra “abstraction layer” in order to prevent future changes from breaking the plugin, but after your new commit I’ll use the new parameter.
Makes sense. I haven’t tested using App.hotkey_actions["prev_article_noscroll"]() for this, but I have been using Headlines.move(key, {noscroll: true}); in that place (see (1)), as synchronous code, without any issues for many months.
Shoot, yes, I should have checked the code better concerning this. Thanks for that!
I tested using Headlines.move("prev", {force_previous: true}) with the latest commit and this works nicely. Thank you!
Commit 2ca25f72c7
Using action in title with hotkey (Ctrl-Up) in combined mode.
Observed behavior:
If headline of the current article is over the top of the screen - the view is scrolled UP to get the headline to the top of the screen.
If the headline is on the top of the screen - previous article gets selected.
If the headline is below the top of the screen - the view is scrolled DOWN to get the headline to the top of the screen.
And if the view can not be scrolled DOWN in such a way (i.e. feed doesn’t have enough articles to be able to scroll down) - nothing happens.
Desired behavior:
If the headline of selected article is below the top of the screen and screen can’t be scrolled DOWN - get previous article selected.
Possible place of needed fix: js/Headlines.js, lines 850-854.
I’m sure everyone is sick and tired of my ongoing impotent attempts to make this arguably primitive thing work “right” but here comes another changeset:
First of all, smooth scrolling is no more.
Second, I’ve tried to make moving between headlines work a bit more intuitive but I’m not sure if I was successful. This should also fix the above issue with jumping down to move up.
That gap by #toolbar-frame_splitter looks a bit, especially with the scroll bar not connecting to #toolbar-frame.
As for the smooth scrolling, after changing line_offset to 250, I kinda liked it. I do think most people have the smooth scrolling setting of their browsers set to enabled, so they expect it. That being said, I have smooth scrolling disabled in my browser, so in the end I don’t care that it’s gone.
oh oops, that was just me deleting too many lines from CSS by mistake. i’ve added this rule back, thanks for noticing.
that was why this was initially added, unfortunately there’s simply too many hacks involved to keep it, at least with that implementation.
e: additionally, i’ve updated some flags in Headlines.move() so if someone is dissatisfied with the way this works now it should be easy to go back to any variation of previous behavior using custom keybinds.
Yeah I did a quick search and honestly, it baffled me that one can apparently can’t let the browser decide (smooth-scroll: auto does not mean what you’d expect), nor is there a JavaScript solution to find out whether the browser smooth scroll setting is enabled or not. Strange stuff…