RomanSixty / Feed-on-Feeds

FeedOnFeeds is a lightweight server-based RSS feed aggregator and reader
http://feedonfeeds.com/
GNU General Public License v2.0
60 stars 15 forks source link

'j' paging-down gets stuck on longer entries now #34

Closed fluffy-critter closed 5 years ago

fluffy-critter commented 5 years ago

I don't know how much of this is due to the recent-ish migration away from prototype or some of the styling changes or whatever, but now when I press j on some feed items, the paging stops working partway through.

One example entry that causes the problem is https://99percentinvisible.org/episode/raccoon-resistance/ which is on a blog which often has quite long posts. It seems that all of their posts cause this problem. On some of them my browser's developer console gets errors like:

TypeError: null is not an object (evaluating 'itemElement.nextElementSibling.id') on fof.js:261

I haven't looked at the new code yet but I suspect there's been some DOM change which causes this quirk.

It happens in Safari, and I haven't tried it in other browsers yet.

J paging (to mark and skip to the next entry) continues to work, at least.

fluffy-critter commented 5 years ago

Hm, it seems that paging past the last entry also no longer triggers the "Mark all flagged items read?" popup.

RomanSixty commented 5 years ago

I stumbled upon the keyboard navigation feature when I made my changes to get rid of prototype. I tried to keep all the functionality but it was more or less a blind run since I'm not using the feature. So I may have missed some hopefully minor feature (like the one mentioned in your latest comment). I may look into it when I have some time to spare, except if you fix it first ;)

RomanSixty commented 5 years ago

However I may have found why keyboard navigation using j broke with large items. Take a look into 3a15d393efd3aa47f1a09381f8ebfea1cb1b3aa1

fluffy-critter commented 5 years ago

I'd consider keyboard navigation to be pretty major; I use it all the time, and having it suddenly break on me was a bit concerning. :)

I'll try out that change and see if it fixes my problem. Thanks!

fluffy-critter commented 5 years ago

Hmm, it still doesn't work quite right: lowercase j is now paging down a whole entry at a time (which is what capital J does normally) and it's not showing the "mark all flagged read?" prompt after paging past the last one. That change alone shouldn't be causing that sort of difference so maybe I just need to go through and rework the keyboard paging code myself...

RomanSixty commented 5 years ago

I found the "mark all flagged read" stuff, I migrated it when refactoring. However the bug with large items was also migrated from the old code, so it probably was already broken... :)

However I did some more refactoring and now I'm getting the prompt after the last item. See c7055e241398438f8db6edea88cee87a72f79770

fluffy-critter commented 5 years ago

It was definitely not broken from the previous time I merged from you. :) Looks like that was way back in May, at 0da2b162d6d2ed415778562b125083a2461f46a2.

Anyway I'll try this new change out!

fluffy-critter commented 5 years ago

Okay, this does at least restore the prompt, but the paging behavior is still very different than how it used to be - it's paging a whole item at a time, rather than scrolling down within a single item. So j and J are still behaving mostly the same (except J doesn't show the prompt, which is the old behavior which IMO was also broken).

Note that the documented behavior (from the help text) is:

j : Scroll current item, flag if at end of item, move to next item. J : Flag current item, move to next.

And that's a behavior I very much want to preserve; it's great for quickly reading articles without completely skipping them, and is also way easier on my wrists than not using the keyboard shortcuts.

Basically it should be scrolling by the screen height, not the item height.

fluffy-critter commented 5 years ago

Anyway the major problem has been fixed so I'll close this issue, and I'll hack on the actual paging behavior to get it nice again on my own. :)

As always, thanks!