Skip to content

[ReaderLink, Dispatcher] add toggle for tap-to-follow links#13469

Merged
Frenzie merged 3 commits into
koreader:masterfrom
Commodore64user:toggle-tap-link
Apr 5, 2025
Merged

[ReaderLink, Dispatcher] add toggle for tap-to-follow links#13469
Frenzie merged 3 commits into
koreader:masterfrom
Commodore64user:toggle-tap-link

Conversation

@Commodore64user

@Commodore64user Commodore64user commented Mar 27, 2025

Copy link
Copy Markdown
Member

what's new

  • Consolidated the enabled_func for footnote popup settings into a single location to avoid redundancy in frontend/apps/reader/modules/readerlink.lua.
  • Added a new function onToggleTapLinks to toggle the tap-to-follow links setting and display a notification of the current status in frontend/apps/reader/modules/readerlink.lua.
  • Included the new toggle_tap_links event in the settings list and dispatcher menu order in frontend/dispatcher.lua.

screenshots

closes #13381


This change is Reviewable

Comment thread frontend/dispatcher.lua
follow_nearest_internal_link = {category="arg", event="GoToInternalPageLink", arg={pos={x=0,y=0}}, title=_("Follow nearest internal link"), reader=true},
select_prev_page_link = { category="none", event = "SelectPrevPageLink", title=_("Select previous link in current page"), reader=true, condition=not Device:isTouchDevice()},
select_next_page_link = { category="none", event = "SelectNextPageLink", title=_("Select next link in current page"), reader=true, condition=not Device:isTouchDevice()},
toggle_tap_links = {category="none", event="ToggleTapLinks", title=_("Toggle tap-to-follow links"), reader=true, condition=Device:isTouchDevice()},

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.

That can't be right. ;-) Shift+LPgBack/Shift+LPgFwd to select links, enter to tap.

@Commodore64user Commodore64user Mar 27, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think you might be confusing two different things, this is the touch setting to tap on links/footnotes to follow them. There is a menu option (screenshot 2, top one) in the settings but no dispatcher action.

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.

It says condition=Device:isTouchDevice(), does it not?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, that is correct. This is only meant for touch devices.

if what you are getting at is that it should exist everywhere, then, there is no reason for it, you can’t accidentally trigger it on NT.

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.

The shouldn't you change the original to behave as such before adding a discrepancy?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah, also as a remainder, all these settings are under “taps and gestures” which is a touch only menu.

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 see. Footnotes in popup does still seem relevant but given that in-page footnotes exist it shouldn't matter much in practice.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Footnotes in popup does still seem relevant.

oh yeah... I hadn't thought about that, I'll see to it another time. one can still follow the links though, or do in-page footnotes as you correctly point out.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

not really sure I understand what you were talking about.
But you can't kill usage of footnote popups just because there is the fallbaack option of using in-page footnotes.
Footnote popup is actually the fallback when in-page footnotes can't be made to work.
And the prefered footnote viewing option for a lot of users.

@Commodore64user Commodore64user Mar 27, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@poire-z we are not killing anything. We just realised that the setting is not available on NT altogether at time of writing. I then committed to fixing it later since I'll have to think where to best place it and check if there are other settings locked behind isTouchDevice anywhere else.

@poire-z

poire-z commented Mar 27, 2025

Copy link
Copy Markdown
Contributor

Do we already disable a full submenu under some conditions ?
(It feels a bit unwelcome to me - it could be allowed to enter it and see what's there, and see it is all disabled - but I just require consistency :)).

@Frenzie

Frenzie commented Mar 27, 2025

Copy link
Copy Markdown
Member

Yes, I'm also not the biggest fan of disabling the submenu and not being allowed to see. ymmv I suppose

@Commodore64user

Commodore64user commented Mar 27, 2025

Copy link
Copy Markdown
Member Author

What’s the point of seeing everything greyed out? It’s like walking into a shopping centre only to find out that all the shops are closed.

@poire-z

poire-z commented Mar 27, 2025

Copy link
Copy Markdown
Contributor

So you can see what kind of shops there are, and can come back tomorrow morning ?

@Commodore64user

Commodore64user commented Mar 27, 2025

Copy link
Copy Markdown
Member Author

That's silly, it should be closed so i don’t waste my time going in there twice. I'll just walk past it today and go there the next day.

@poire-z

poire-z commented Mar 27, 2025

Copy link
Copy Markdown
Contributor

You could still not waste your time even if it is opened (and not care how I waste my time) - you have a will of your own :)

@poire-z

poire-z commented Mar 27, 2025

Copy link
Copy Markdown
Contributor

Also curious and no idea: how does Menu search work with items in a fully disabled submenu?
Does it show the items in the results?
Does "Walk me there" walk you into the closed shopping center?

@mergen3107

Copy link
Copy Markdown
Contributor

So you can see what kind of shops there are, and can come back tomorrow morning ?

Actually, yeah :D

I had a few occasions when I saw a setting, but I am not sure where it was exactly.

Then I go around all settings, in different book types and FM, then find it. (I think it was panel zoom or something back in the days when I was sillier to understand that it is only for mupdf)

It was before menu search was introsuced that saved A LOT of time, so thank you for that!

@Commodore64user

Copy link
Copy Markdown
Member Author

Also curious and no idea: how does Menu search work with items in a fully disabled submenu? Does it show the items in the results? Does "Walk me there" walk you into the closed shopping center?

a quick test shows that it bypasses the enabled_func status and in fact takes you all the way to the setting. As for whether that is problematic or not, that is my favourite kind of problem… someone else’s… ;)

@poire-z

poire-z commented Mar 27, 2025

Copy link
Copy Markdown
Contributor

It's still your problem to go look everywhere if there are other such submenu with fully disable'able subitems - and if any, see how your change of paradigmenu would work with them.

@Commodore64user

Copy link
Copy Markdown
Member Author

It's still your problem to go look everywhere if there are other such submenu with fully disable'able subitems - and if any, see how your change of paradigmenu would work with them.

We already told you, yes, there are lots of them. Inside “taps and gestures” alone there is “Scrolling” for example. In this particular case, there are no issues.

@poire-z

poire-z commented Mar 27, 2025

Copy link
Copy Markdown
Contributor

There are lots of them that what ? That get disabled ? That don't get disabled with all subitems disabled?
I see Scrolling> is disabled when not in scroll mode - so your change won't be a first.
Are there of the other kind?

@Commodore64user

Copy link
Copy Markdown
Member Author

As far as i am aware, this is the only submenu (famous last words, i am sure someone will prove me wrong) that disables the children and not the parent. More examples of parents being disabled are found in “status bar” menu, “sleep screen” menu.

@poire-z

poire-z commented Mar 27, 2025

Copy link
Copy Markdown
Contributor

OK then, that's what I was asking for all along.

@Commodore64user

Copy link
Copy Markdown
Member Author

btw @poire-z, I am not sure why I am bringing this up here or now, but have you seen this? https://www.brailleinstitute.org/freefont/ seems like something that could be implemented here to help those with vision impairments.

@Frenzie

Frenzie commented Mar 28, 2025

Copy link
Copy Markdown
Member

It's easy to add your own fonts. I'm not sure if it's worth adding any more out of the box.

You could envision some kind of font downloader plugin though.

@poire-z

poire-z commented Mar 28, 2025

Copy link
Copy Markdown
Contributor

I'm not sure either :) And I'm already quite busy helping those with Touch impairments :)

But I'm not sure what "implementation" you're thinking about - it seems this is just a font that users insterested can just download and put on their device - and we'd have nothing to implement ?

@Commodore64user

Copy link
Copy Markdown
Member Author

I'm not sure either :) And I'm already quite busy helping those with Touch impairments :)

Is that so? I don't think there is a PageBrowser yet... ;)

But I'm not sure what "implementation" you're thinking about - it seems this is just a font that users insterested can just download and put on their device - and we'd have nothing to implement ?

I don’t know, i just thought it would be something interesting to mention in case you wanted to try something with it.

@Frenzie Frenzie added this to the 2025.03 milestone Apr 5, 2025
@Frenzie Frenzie merged commit 68f3b34 into koreader:master Apr 5, 2025
@Commodore64user Commodore64user deleted the toggle-tap-link branch April 5, 2025 15:03
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.

FR: Add "Toggle tap to follow link" as a possible action for a gesture

4 participants