Combined (unexpanded) mode has to go

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.

Count me as another who really really enjoyed the combined mode, and am missing that comparatively (to me) clean UI (one article open at a time, no additional columns/rows).

FWIW i’m playing with this for custom css to unexpand the inactive headlines. It’s not a perfect user experience (i can’t actually see a spot to click on one of these “unexpanded” articles, but up/down selects one fine).

div.cdm.active > div.content {
display: block;
}

div.cdm:not(active) > div.content {
display:none;
}

@atrus,

You write in the past tense as though combined mode is gone, along with your custom CSS it makes me think you don’t know that fox added it back.