Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Duplicated options on Non-touch #11747

Closed
Commodore64user opened this issue May 4, 2024 · 8 comments · Fixed by #11750
Closed

Duplicated options on Non-touch #11747

Commodore64user opened this issue May 4, 2024 · 8 comments · Fixed by #11750
Labels
NT Non Touch devices
Milestone

Comments

@Commodore64user
Copy link
Contributor

  • KOReader version: 2024.03.1-55 (on older versions too)
  • Device: Kindle 4/5 (non-touch in general)

Issue

There is one option duplicated in the menus, one comes from --#8877 and later #8914-- the other not sure.
Both NEW: Long-press on text and Select on text appear to be the exact same thing with different names.

Steps to reproduce

Open any book, go to "page with bookmark > NEW: Long-press on text" the other is found under "page with text > Select on text"

@Frenzie
Copy link
Member

Frenzie commented May 4, 2024

The former isn't supposed to show on non-touch devices. Maybe something like this would do the trick?

diff --git a/frontend/apps/reader/modules/readerhighlight.lua b/frontend/apps/reader/modules/readerhighlight.lua
index e80998717..abba38f1f 100644
--- a/frontend/apps/reader/modules/readerhighlight.lua
+++ b/frontend/apps/reader/modules/readerhighlight.lua
@@ -593,6 +593,7 @@ Except when in two columns mode, where this is limited to showing only the previ
     if not Device:isTouchDevice() and Device:hasDPad() then
         menu_items.selection_text = util.tableDeepCopy(menu_items.long_press)
         menu_items.selection_text.text = _("Select on text")
+        menu_items.long_press = nil
     end
 
     -- main menu Search

@Frenzie Frenzie added the NT Non Touch devices label May 4, 2024
@Commodore64user
Copy link
Contributor Author

that works! should I PR this or will you?

@Frenzie
Copy link
Member

Frenzie commented May 4, 2024

If you have time to, please do.

@Commodore64user
Copy link
Contributor Author

Commodore64user commented May 4, 2024

I found the [guilty] offender here #8608. 😂

@Commodore64user
Copy link
Contributor Author

Commodore64user commented May 7, 2024

The former isn't supposed to show on non-touch devices. Maybe something like this would do the trick?

diff --git a/frontend/apps/reader/modules/readerhighlight.lua b/frontend/apps/reader/modules/readerhighlight.lua
index e80998717..abba38f1f 100644
--- a/frontend/apps/reader/modules/readerhighlight.lua
+++ b/frontend/apps/reader/modules/readerhighlight.lua
@@ -593,6 +593,7 @@ Except when in two columns mode, where this is limited to showing only the previ
     if not Device:isTouchDevice() and Device:hasDPad() then
         menu_items.selection_text = util.tableDeepCopy(menu_items.long_press)
         menu_items.selection_text.text = _("Select on text")
+        menu_items.long_press = nil
     end
 
     -- main menu Search

But, with this line added, we do not need tableDeepCopy anymore.

@Commodore64user
Copy link
Contributor Author

I tried to get it to work without tableDeepCopy and every approach I try fails... I am sure it will be something silly and obvious once it is solved but for now, I'm stuck.

@hius07
Copy link
Member

hius07 commented May 11, 2024

You just have menu_items.selection_text = menu_items.long_press, right?

@Commodore64user
Copy link
Contributor Author

You just have menu_items.selection_text = menu_items.long_press, right?

that was the very first thing I tried, and it didn't work for some reason. But now it does, weird.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NT Non Touch devices
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants