Skip to content

Add "Open (opened+unopened) random document" on long-press - #13160

Merged
Frenzie merged 1 commit into
koreader:masterfrom
artjomsR:feat/open_any_random_document
Apr 23, 2025
Merged

Frenzie merged 1 commit into
koreader:masterfrom
artjomsR:feat/open_any_random_document

Conversation

@artjomsR

@artjomsR artjomsR commented Jan 29, 2025 •

Copy link
Copy Markdown
Contributor

Would help with reading texts in parallel, especially when it's difficult to decide which one to pick next :)

I've considered grouping both Random functions into a folder, but that would introduce friction with using them.


This change is Reviewable

@hius07

hius07 commented Jan 29, 2025

Copy link
Copy Markdown
Member

Looks good.

@artjomsR

Copy link
Copy Markdown
Contributor Author

Is there anything I can do to support this PR? It's been a month+ with no activity, whereas other PRs are getting at least some action. It's not a big change

function FileManager:openRandomFile(dir)
local match_func = function(file) -- documents, not yet opened
return DocumentRegistry:hasProvider(file) and not BookList.hasBookBeenOpened(file)
function FileManager:openRandomFile(dir, is_unopened)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd name it something like unopened_only or skip_opened or a variety of other things that don't imply it's a property of a file. Besides that lgtm.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I've updated the PR to rename the parameter

@artjomsR
artjomsR force-pushed the feat/open_any_random_document branch from 7d0c946 to 4867cb7 Compare March 24, 2025 17:27
@poire-z

poire-z commented Mar 24, 2025

Copy link
Copy Markdown
Contributor

I've considered grouping both Random functions into a folder, but that would introduce friction with using them.

You could have put in on long-press and hide the "feature" a bit :)

(Personally, I find this kind of "feature" a joke/toy. I missed (or forgot) the addition of the existing "Open random document". Seeing 2 crappy button in there makes me sad.)

@artjomsR

Copy link
Copy Markdown
Contributor Author

You could have put in on long-press and hide the "feature" a bit :)

What do you mean exactly by a long-press here? Do you mean to display another context menu when long-pressing Open random document (effectively creating a nested context menu)?

(Personally, I find this kind of "feature" a joke/toy. I missed (or forgot) the addition of the existing "Open random document". Seeing 2 crappy button in there makes me sad.)

It's useful to me personally, and other people as well, seeing how there were contributions to this feature previously. It shouldn't matter if the feature may not be perceived as serious, as long as it provides value / enjoyment to users (which it clearly does)

@poire-z

@artjomsR
artjomsR requested a review from Frenzie March 25, 2025 13:50
@Frenzie Frenzie added this to the 2025.03 milestone Mar 25, 2025
@poire-z

poire-z commented Mar 25, 2025

Copy link
Copy Markdown
Contributor

What do you mean exactly by a long-press here? Do you mean to display another context menu when long-pressing Open random document (effectively creating a nested context menu)?

Just have the most useful (ah ah) behaviour on tap, and the less useful on long-press, no need to advertise both. We have that in frontend (ie. Book map, long-press brings the "overview" mode).
Most features anyway are available and used via a gesture, where both actions are available there.
The same could work for this "open random document" feature.

It shouldn't matter if the feature may not be perceived as serious, as long as it provides value / enjoyment to users (which it clearly does)

2 years ago, some user came here with a PR that added in that same menu 3 options:
Show fishes swimming on the screen while reading
Show birds flying on the screen while reading
Select opacity of fishes and birds
I obviously got the same reaction as here : it's ridiculous !
But that user assured me that he - and his mother - would be provided value and enjoyment with having that, a little animation over an otherwise static screen page. I understand that young people get sick nowadays when nothing on a screen changes for more than 20 seconds. But still, it's ridiculous, and I stuck to my position: it was rejected.
Later on, there has been tensions between that user and his mother, and I still carry the guilt of my decision...
But because of my decisions, you (and the person who added "Open random document" in here) got to have the room in that menu to add your menu item.
One day, when we'll need to add more useful features in that menu, we'll probably hate you for having added it :)

The above is a fiction, but you get my idea :) We still have to do some policing work so KOReader doesn't becomre a crappy stack of features used by 2 people.
I stated my sentiment, @Frenzie approved, so OK I guess.

Having a quick look at the code, I assume "Open random document" opens a document from the current folder (and under?), and it won't just open any random document from the library root. So, it's not as crappy as I initially thought.
(And I find it crappy in that if that random document does not please you, or is big, you'll have to wait 40s for it to load to be able to choose another document - and you'll have to clean its sidecar and the book from history...)

@Frenzie

Frenzie commented Mar 25, 2025

Copy link
Copy Markdown
Member

I stated my sentiment, @Frenzie approved, so OK I guess.

I don't disagree with you, but it's seems small and contained. And yes, it's folder-bound.

@artjomsR

Copy link
Copy Markdown
Contributor Author

And I find it crappy in that if that random document does not please you, or is big, you'll have to wait 40s for it to load to be able to choose another document

It prompts you with the name of the document it's about to open, so you can simply skip undesired documents, so that should help with this issue

@hius07

hius07 commented Apr 6, 2025

Copy link
Copy Markdown
Member

I agree that the current "Open random document" button can be kept as is, and the new functionality (include opened documents) to be added to long-pressing on the button.

@Frenzie Frenzie modified the milestones: 2025.03, 2025.05 Apr 6, 2025
@artjomsR
artjomsR force-pushed the feat/open_any_random_document branch 2 times, most recently from 46c5af7 to 1958285 Compare April 14, 2025 15:54
@artjomsR

Copy link
Copy Markdown
Contributor Author

@hius07 I've updated it

@hius07

hius07 commented Apr 14, 2025

Copy link
Copy Markdown
Member

You've changed the default action (on tap).

@artjomsR

Copy link
Copy Markdown
Contributor Author

@hius07 Yes, if you were to combine both actions, then I think it makes sense to change the default: opening any document would be used more often (during the same time period) compared to the unopened document. I.e. the user eventually runs out of unopened documents, so that functionality becomes used less and less

On the flip side, you could be using open any document functionality several times per reading session, every session, so it doesn't make sense to prolong invoking this functionality

@hius07

hius07 commented Apr 14, 2025

Copy link
Copy Markdown
Member

the user eventually runs out of unopened documents

He adds new books regularly.

I have no personal preferences (not using this feature). Just thinking about the long time default behavior that people might be used to.

@artjomsR

Copy link
Copy Markdown
Contributor Author

@hius07

He adds new books regularly.

Fair, but my point still stands that any random document has the potential to be used more often just by its nature. Software changes all the time, so I don't see how the user wouldn't adapt here as well, it's not a big change. I don't think it's logical to keep the non-optimal default behaviour just because it was introduced first.

@Frenzie Frenzie changed the title Add "Open (opened+unopened) random document" plus option Add "Open (opened+unopened) random document" on long-press random document Apr 23, 2025
@Frenzie Frenzie changed the title Add "Open (opened+unopened) random document" on long-press random document Add "Open (opened+unopened) random document" on long-press Apr 23, 2025
@Frenzie

Frenzie commented Apr 23, 2025

Copy link
Copy Markdown
Member

What's the old and what's the new behavior now precisely? (Or: write a commit message please ;-P)

@artjomsR

Copy link
Copy Markdown
Contributor Author

@Frenzie

Before / master

On tap: open unopened random document

After / this PR

On tap: open any random document
On hold press: open unopened random document

@Frenzie

Frenzie commented Apr 23, 2025

Copy link
Copy Markdown
Member

What's the rationale for making the unexpected behavior the default?

@artjomsR

Copy link
Copy Markdown
Contributor Author

@Frenzie Basically because you'd use opening any doc a few times during a reading session, but unopened doc - only occasionally. And due to the nature of the Open any doc, it seems logical to make its invocation to be fastest

From my comment above - opening any document would be used more often (during the same time period) compared to the unopened document. I.e. the user eventually runs out of unopened documents, so that functionality becomes used less and less

On the flip side, you could be using open any document functionality several times per reading session, every session, so it doesn't make sense to prolong invoking this functionality. I don't think it's logical to keep the non-optimal default behaviour just because it was introduced first.

@Frenzie
Frenzie merged commit 89f7f5a into koreader:master Apr 23, 2025
@Frenzie

Frenzie commented Apr 23, 2025

Copy link
Copy Markdown
Member

I'm not quite convinced, but I suppose I'd really expect it to avoid the previous x results, which has some overlap with previously opened but is something quite different nonetheless.

0xstillb pushed a commit to 0xstillb/koreader-thai that referenced this pull request May 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants