Combined (unexpanded) mode has to go

[Ajurna]

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

[Glandos]

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) {

[fox]

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

[Glandos]

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.

[tsimmons]

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;

[tsimmons]

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!

[fox]

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

[tsimmons]

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.

[Glandos]

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.

[fox]

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

[fox]

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

[Glandos]

@fox

if (Article.getActive() != id) {

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.

[atrus]

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;
}

[JustAMacUser]

@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.

[atrus]

Maybe I’m confused, and I’ve been using the combined-unexpanded mode long enough I don’t know how to set it up again. I updated from TTRSS git from a few months back to today, and saw all articles expanded in the combined view, and no obvious way to have only the active article expanded (as my CSS does).

Is there some option/toggle somewhere I’m missing about that?

[JustAMacUser]

@atrus

Is there some option/toggle somewhere I’m missing about that?

Preferences.

/20chars

[fox]

check out hotkeys help (f c and f C)

[ltGuillaume]

I was trying to find a solution for keyboard-driven navigation that allows skipping certain articles to read later, i.e. navigating to and opening the high-priority articles first without opening/expanding the articles in between (and thus leaving them marked as unread).

Currently the hotkey functions next_article_noexpand/prev_article_noexpand" (“Move to article (don’t expand or mark read)”) are still available, but actually do mark the article as read (xhrPost: Object { op: "rpc", method: "catchupSelected", ids: "***", cmode: 0 } is when navigating).

Similarly, the hotkey function toggle_expand doesn’t seem to work anymore (related: Collapsing article). I understand that this is probably because these functions are “remnants” of the previous implementation of the unexpanded combined mode (Combined (unexpanded) mode has to go - #65 by Bob). Could this be tackled, so that the method of navigation as described above can be (re)introduced?

tt-rss commit 088fcf8131

[fox]

as far as i remember it was either remove this altogether or bring back a limited implementation which would not affect so much core code, which is what i did.

just because it’s six months later i’m not going to suddenly change my mind and start adding back complexity like separate active / unread tracking etc.

you should be able to implement this via plugins, if necessary by replacing methods on JS side. this way this code would be your responsibility and i won’t have to maintain it.

e: _noexpand actions are likely used in three panel mode for quick navigation which is not related to unexpanded stuff.

[ltGuillaume]

@fox

e: _noexpand actions are likely used in three panel mode for quick navigation which is not related to unexpanded stuff.

Three panel mode is exactly where these two hotkey functions do not open the articles (as intended), but do mark them as read.