Combined (unexpanded) mode has to go

unless i’m missing something you’re describing three panel mode?

I may indeed be confused. Yes, I am referring to 3-panel mode. Sorry, I didn’t know the name of it.

If people could navigate the article list in 3-panel mode without triggering the article content panel, and instead only manually open the article content panel when they want to (they can already close it), the result is very close to combined unexpanded mode for all practical purposes.

I apologize if the above can already be done, but I cannot find the kybd shortcuts for the following 2 actions which would be required:

  • navigate the article list without opening the content panel
  • manually open the content panel. The content panel can be closed through the ‘aq’ shortcut, but it seems that the only way to reopen it is to navigate away and then back to the article.

It may be a problem with my installation as I see shortcuts in the list which seem like they might perform these actions, but I cannot get them to work.

Just wandering into this thread, and being a combined unexpanded user it was quite the roller coaster <3

just you wait until i merge js-objects into master

Guess I should wait a couple weeks to pull so the dust can settle.

Some of the keyboard shortcuts appear to be broken with the recent updates, i’ve got the following set in a plugin, but they’re not working…

$hotkeys["n"] = "next_article_noexpand";
$hotkeys["p"] = "prev_article_noexpand";
$hotkeys["o"] = "toggle_expand";

I’m guessing they were removed during the purge… can they be added back in?

since there’s no separate expanded state right now (and there won’t be, not limited to this mode), i’m not sure how this should work
i’ll think about it

Thank for re-adding combined mode, @Fox, much appreciated.

thanks as well for re-adding it. i also send a donation as thanks :slight_smile:

I have a small patch for combined mode. This is to avoid the call to cdmScrollToArticleId each time we click in an article (e.g. in the intermediate part).
I see that there are a lot of changes in js/viewfeeds.js, so I just copy/paste it here for now. I will open a pull request if it is still needed:

diff --git a/js/viewfeed.js b/js/viewfeed.js
index 73d135874..e8e9efee8 100755
--- a/js/viewfeed.js
+++ b/js/viewfeed.js
@@ -794,8 +794,13 @@ function editArticleTags(id) {
 }
 
 function cdmScrollToArticleId(id, force) {
+       const active = "RROW-" + getActiveArticleId()
+       const current = "RROW-" + id;
+       if (active === current && !force) {
+               return;
+       }
        const ctr = $("headlines-frame");
-       const e = $("RROW-" + id);
+       const e = $(current);
 
        if (!e || !ctr) return;
 
@@ -1106,11 +1111,11 @@ function cdmClicked(event, id, in_body) {
                openArticleInNewWindow(id);
        }
 
-       setActiveArticleId(id);
-
        if (!getInitParam("cdm_expanded"))
                cdmScrollToArticleId(id);
 
+       setActiveArticleId(id);
+
        //var shift_key = event.shiftKey;
 
        /* if (!event.ctrlKey && !event.metaKey) {

I’m sorry but at this point you’ll have to rebase on js-objects and go from there if it’s still relevant.

Don’t be sorry: you’re working hard for everyone else. I posted it for interested people. I will update it as needed on js-objects once its barely done.

To disable automatic scrolling (it drives me a bit batty when expanding/collapsing headlines) you can comment line 237 of Article.js (this is running on git 3318555167):

//ctr.scrollTop = parseInt(e.offsetTop) - 4;

I’d also like to say that after a couple of tweaks to my custom theme, and just a couple of other minor issues, my enjoyment of TT-RSS continues unabated! Thanks so much Fox for your work on something that makes it easy to keep up with tons of Internet content in an intuitive and efficient manner!

i agree jumping could be annoying but this way article would go offscreen when you move with keyboard shortcuts, lol

there’s probably a better way to deal with this

e: another issue is as follows, which is part of why scrolling is there

  1. you have an active article somewhere
  2. you scroll down and click on something
  3. previous article gets hidden and headlines buffer jumps up
  4. your current article is offscreen somewhere

this stuff is just so much more consistent in expanded and three panel mode

Actually, I’m not seeing that behavior with that line commented. Basically, the headlines stay where they are … If I open a large article, then scroll down several pages then open a new article, the headline appears pinned right where I clicked and the article expands in place, below where the headline is. :man_shrugging:

It is still relevant. At least for me.
Currently, in combined mode (unexpanded), when I’m reading an article, I can’t select text without getting scrolled to the beginning. Or I can’t click on links without having the same scrolling to the beginning.

Here is my patch:

diff --git a/js/Article.js b/js/Article.js
index f2c4007f5..e4dfb744f 100644
--- a/js/Article.js
+++ b/js/Article.js
@@ -225,8 +225,13 @@ define(["dojo/_base/declare"], function (declare) {
                        dialog.show();
                },
                cdmScrollToId: function (id, force) {
+                       const active = "RROW-" + this.getActive();
+                       const current = "RROW-" + id;
+                       if (active === current && !force) {
+                               return;
+                       }
                        const ctr = $("headlines-frame");
-                       const e = $("RROW-" + id);
+                       const e = $(current);
 
                        if (!e || !ctr) return;
 
diff --git a/js/Headlines.js b/js/Headlines.js
index 8895399b2..f006af673 100755
--- a/js/Headlines.js
+++ b/js/Headlines.js
@@ -16,11 +16,11 @@ define(["dojo/_base/declare"], function (declare) {
                                        Article.openInNewWindow(id);
                                }
 
-                               Article.setActive(id);
-
                                if (!App.getInitParam("cdm_expanded"))
                                        Article.cdmScrollToId(id);
 
+                               Article.setActive(id);
+
                                return in_body;
 
                        } else {

Of course, it is a quick and dirty patch. It might not feel your expectations. I can open a pull request if like.

this looks alright to me on the first glance, please file a PR.

now that i think about it, i think it would be easier to do this instead:

index 8895399b..496e315a 100755                                               
--- a/js/Headlines.js                                                         
+++ b/js/Headlines.js                                                         
@@ -16,10 +16,12 @@ define(["dojo/_base/declare"], function (declare) {       
                                        Article.openInNewWindow(id);          
                                }                                             
                                                                              
-                               Article.setActive(id);                        
+                               if (Article.getActive() != id) {              
+                                       Article.setActive(id);                
                                                                              
-                               if (!App.getInitParam("cdm_expanded"))        
-                                       Article.cdmScrollToId(id);            
+                                       if (!App.getInitParam("cdm_expanded"))
+                                               Article.cdmScrollToId(id);    
+                               }                                             
                                                                              
                                return in_body;                               
                                                                              

it should achieve the same result without introducing uncertainty into scrollToId()
maybe this breaks something though, i’ll keep your PR for reference for the time being

This is way better than my previous patch. It is:

  • simpler
  • cleaner
  • a real fix.

Regarding the last point, I found that my solution was buggy in case of activating a new article while another one was already active: scrolling was not done as expected. Whereas, in your solution, it fits. I stick with your patch for a while, and tell you if I experienced something bad.