Skip to content

[DictQuickLookup] NT: add text selection to the dictionary widget#13232

Merged
Frenzie merged 25 commits into
koreader:masterfrom
Commodore64user:dict-text-select
Feb 17, 2025
Merged

[DictQuickLookup] NT: add text selection to the dictionary widget#13232
Frenzie merged 25 commits into
koreader:masterfrom
Commodore64user:dict-text-select

Conversation

@Commodore64user

@Commodore64user Commodore64user commented Feb 10, 2025

Copy link
Copy Markdown
Member

what's new

This pull request is dedicated to enhancing the user experience for non-touch devices. At present, when reading a dictionary definition on said devices, users have limited options for performing a meta-search. While those with a keyboard (hasKeyboard) can effortlessly begin typing a new word to look up, the process is not as seamless on devices like the Kindle 4 or those with minimal keys (hasFewKeys). To address this challenge, and from the creator of Hotkeys, I am delighted to introduce to the [inter]national stage…

usage

Initially, when you open the dictionary widget from the reader, FocusManager will function as usual. On Kindle 4 or devices with a keyboard, I have assigned the shortcuts screenkb + up/down or shift + up/down to initiate text selection (as a remainder, on kindle text selection in reader is initiated with up/down so it is fairly similar). At this point, FocusManager is disabled, allowing the cursor keys (and press) to control the now-visible crosshairs. Pressing back should stop text selection and restore FocusManager’s control of the widget. (this is the case for the first three screenshots)

enhancements to DictQuickLookup:

  • Added a text selector feature for non-touch devices, allowing users to select text using the D-pad. This includes new methods such as onStartTextSelectorIndicator, onStopTextSelectorIndicator, and onMoveTextSelectorIndicator to manage the indicator's behaviour.
  • Refactored key event registration into a new method registerKeyEvents to clean up the initialisation process and consolidate key event handling logic.
  • Modified the paintTo method of ScrollHtmlWidget and ScrollTextWidget to draw the text selector indicator on top of the content.

others:

  • Introduced DoubleSpinWidget for adjusting crosshairs speed settings separately for reader and dictionary modes. This replaces the previous single SpinWidget.
  • Updated FocusManager to include a key_events_enabled flag, allowing temporary disabling of key events during text selection.

screenshots

focus manager in control of widget vs text selection in charge


This change is Reviewable

@Frenzie

Frenzie commented Feb 10, 2025

Copy link
Copy Markdown
Member

Could you also describe the behavior a bit? How do you trigger the crosshairs?

@Commodore64user

Commodore64user commented Feb 10, 2025

Copy link
Copy Markdown
Member Author

Could you also describe the behavior a bit? How do you trigger the crosshairs?

Absolutely. Initially, when you open the dictionary widget from the reader, FocusManager will function as usual. On Kindle 4 or devices with a keyboard, I have assigned the shortcuts screenkb + up/down or shift + up/down to initiate text selection (as remainder, on kindle text selection in reader is initiated with up/down so it is fairly similar). At this point, FocusManager is disabled, allowing the cursor keys (and press) to control the now-visible crosshairs. Pressing back should stop text selection and restore FocusManager’s control of the widget. (this is the case for the first three screenshots)

I don't have a direct solution for hasFewKeys (in terms of triggering text selection) but perhaps an extra button could be added to the widget that allows it to be initiated, also, on few keys the left key remains as back key so the crosshairs wraps around the right edge allowing for a full coverage area.

@Commodore64user

Commodore64user commented Feb 11, 2025

Copy link
Copy Markdown
Member Author

some issues

there is one thing I have noticed however, sometimes and really unexpectedly, if try to select something, something else is selected instead, for example. The following two screenshots show where I placed the crosshairs and where I eventually made the selection (first image), only for the selection to occur as seen in screenshot 2. In fact in that occasion, I could not select any other word in that same line except for that one.

I tried to select the same word on kindle voyage and everything works as one would expect, not sure what is going on there. Perhaps _createHighlightGesture is not quite properly calibrated, but I don't think so...

    Screen
+----------------------------------------+
|                                        |
|      DictQuickLookup Window            |
|    +----------------------------+      |
|    |[Menu] DictTitleBar [Close] |      |
|    |----------------------------|      |
|    |    Lookup Word + Icons     |      |
|    |   [Word]     [Edit] [#/N]  |      |
|    |----------------------------|      |
|    |    ScrollTextWidget or     |      |
|    |    ScrollHtmlWidget        |      |
|    |    +----------------+      |      |
|    |    |  Definition   |]<-Scrollbar  |
|    |    |   Content     ||      |      |
|    |    |               ||      |      |
|    |    |   [+]         ||      |      |
|    |    | Indicator     ||      |      |
|    |    |               ||      |      |
|    |    +----------------+      |      |
|    |----------------------------|      |
|    |     Button Table           |      |
|    | [Prev] [Highlight] [Next]  |      |
|    | [Wiki] [Search] [Close]    |      |
|    +----------------------------+      |
|                                        |
+----------------------------------------+

Another issue is wikipedia, it won't work, hopefully some hero will come and save the day ;) I can be your hero baby!

edit: apparently if I untick Justify text suddenly I am abled to select all words normally in that line, weirder

Comment thread frontend/ui/widget/dictquicklookup.lua Outdated
Comment on lines +852 to +863
text_selector = { indicator = nil, }, -- Add this for indicator visibility
highlight_text_selection = true,
html_link_tapped_callback = function(link)
self.html_dictionary_link_tapped_callback(self.dictionary, link)
end,
-- We should override the widget's paintTo method to draw our indicator
paintTo = function(widget, bb, x, y)
-- Call original paintTo from ScrollHtmlWidget
ScrollHtmlWidget.paintTo(widget, bb, x, y)
-- Draw our indicator on top if we have one
if widget.text_selector.indicator then
local rect = widget.text_selector.indicator

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.

You're becoming good ! ... at user patching :)
I'm not sure this kind of user patching hack is welcome in the core codebase (I may have done a few in the custom toc/hidden flows PR, to not spread the infection to many things and keep it localized).

You don't need this crappy text_selector = { indicator = nil, } (a table so you can pass it and access this mutable indicator thingie).
You could as well just access self in here, and have indicator as a property of DictQuickLookup (with maybe a more verbose name).

As for this paintTo user patching, may be you could just pass a post_paintTo_func and check and call it in the ScrollText/HtmlWidgets if it exists (it's touching these other modules, but it's just 3 generic lines added). Dunno. @NiLuJe any preference or better idea ?

@Commodore64user Commodore64user Feb 12, 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 went ahead and followed your suggestion, any comments there?

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 no, it is just wrong. I just noticed it. Terrible job.

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 I could do with some help here, cause I am lost here now. The post_func doesn't draw the scrollbar and "scrolling" through the content doesn't update correctly the box. in short, it's a mess

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.

Well, reading 33e4e0d, I'm not sure I like my suggestion :)
May be just do as you originally did, but set it only when you know you need it, that is when the device is non-touch and has the capabilities. (And don't put the conditions all over again, just set a self.allow_key_selection (or something like that) in :init().)

@Commodore64user Commodore64user Feb 13, 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.

May be just do as you originally did, but set it only when you know you need it, that is when the device is non-touch and has the capabilities. (And don't put the conditions all over again, just set a self.allow_key_selection (or something like that) in :init().)

I'm not sure I follow. I think I do now

Comment thread frontend/ui/widget/dictquicklookup.lua Outdated
Comment on lines +1677 to +1679
--[====[
The following methods are used to handle text selection in the dictionary widget for non-touch devices.
]====]

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.

We don't really need more folklore (you were allowed for the "typed on my commodore64", that's enough :)).

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.

coming from a person that "allegedly" enjoys reading, this is quite disheartening ;)

I only was trying to segregate the nt stuff so it doesn't get in anyone's way

Comment thread frontend/ui/widget/dictquicklookup.lua Outdated
- Requires self.text_widget.text_selector.indicator to not exist
]]
function DictQuickLookup:onStartTextSelectorIndicator()
if not (self.definition_widget and not self.text_widget.text_selector.indicator) then return false end

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.

A bit hard to read condition, I stopped here for a minute trying to reformulate it to see what it checks.

if not self.definition_widget then return false end -- not yet set up
if self.text_widget.text_selector.indicator then return false end -- already started

would read clearer.

Comment thread frontend/ui/widget/dictquicklookup.lua Outdated
Comment on lines +1808 to +1818
-- Update display before changing widget state to clear old position
UIManager:setDirty(self, function()
return "ui", self.definition_widget.dimen
end)
-- Update widget state
self.text_widget.text_selector.indicator = rect
-- Trigger refresh on the ScrollTextWidget/ScrollHtmlWidget itself
-- to ensure the paintTo method that draws the crosshair is called
UIManager:setDirty(self, function()
return "ui", self.definition_widget.dimen
end)

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.

Your comment in the previous functions -- Mark definition widget area as dirty for clean re-draw was more correct than these ones.
You calling here twice setDirty won't trigger anything, they'll just mark the same stuff for refresh for the next time UIManager is given control back. As you continue your cpu-bound work in this function, nothing will happen after the first setDirty.
You may as well just add one setDirty at the end of this function.

Comment on lines +1829 to +1841
--[[
@details This function manages the text selection process and subsequent actions:
- Initiates text selection on first press
- On second press (when selection is complete):
* Processes the selection
* Handles Wikipedia/Dictionary lookup based on hold duration
* Clears highlights after lookup
@note Wikipedia lookup can be toggled with a long hold (≥5 seconds)
@return boolean Returns true if the event was handled, false otherwise
@local variables
_text_selection_started: boolean tracking selection state
_hold_duration: timestamp for tracking hold duration
]]

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.

I've seen this kind of (really verbose) comments in a few @Commodore64user recent PRs. Are they extracted and displayed nicely in the doc @Frenzie ?

(I personally prefer reading the comments in the code, than having such long description at the top, having to glance up and down - I think I mentionned that before.)

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.

Probably not very nicely, but with two dashes it won't show in the docs regardless. I'd agree it should be much briefer to be useful. The internal functionality is internal.

@poire-z

poire-z commented Feb 11, 2025

Copy link
Copy Markdown
Contributor

This pull request is dedicated to enhancing the user experience for non-touch devices.

This PR title is missing a "Non-Touch" element :)
The world is more than the stage you think it is :)

I am delighted to introduce to the [inter]national stage…

The international stage may not really care :/ I apologize on its behalf.

there is one thing I have noticed however, sometimes and really unexpectedly, if try to select something, something else is selected instead
[...]
edit: apparently if I untick Justify text suddenly I am abled to select all words normally in that line, weirder

No idea. I tried understanding the part of your code involved, no clue.

@Commodore64user Commodore64user changed the title [DictQuickLookup] add text selection inside dictionary widget [DictQuickLookup] NT: add text selection to the dictionary widget Feb 11, 2025
Comment thread frontend/ui/widget/dictquicklookup.lua Outdated
Comment on lines +1826 to +1835
if self.is_wiki and selection_widget.highlight_start_idx then
-- For wiki content, extract the selected text using the indices
selected_text = selection_widget.text:sub(
selection_widget.highlight_start_idx,
selection_widget.highlight_end_idx
)
else
-- For dictionary content, highlight_text should contain the complete selection
selected_text = selection_widget.highlight_text
end

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 sure why you need different methods, but you should say why :)
Have you tested it with a non-HTML dict ? Because wikipedia and text dict results should use the same TextBoxWidget.

@Commodore64user Commodore64user Feb 12, 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.

well selection_widget.highlight_text returns nil with wiki. highlight_text is a property of HTMLBoxWidget and wiki doesn't use that.

function ScrollHtmlWidget:init()
self.htmlbox_widget = HtmlBoxWidget:new{

function ScrollTextWidget:init()
self.text_widget = TextBoxWidget:new{

what is a non-html dictionary? ;) I don't think I have one, so no. Should I invert the check then? if self.is_html then selected_text = selection_widget.highlight_text else ... end

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.

You may investigate what have changed and how textboxwidget vs. htmlboxwidget differ. May be they should have a common method to get the selected text (it's possible @TnS-hun changed stuff with the indices and that the selected text is no longer readily available - if this is the case, I'd rather add to each of them a common method to get it, rather than having to handle the differences here like you do).

A non-html dict is one that doesn't have sametypesequence=h in its .ifo. Their results shows no bold/italic/font size differences, like Wikipedia results: plain boring text.
If you don't find any (they should be more common than =h though), just copy one of yours and change in its ifo its name and sametypesequence=h to =x: you'll get the html converted to plain boring text, which should suffice to test.

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.

You may investigate what have changed

in #12948

and how textboxwidget vs. htmlboxwidget differ ...

@Commodore64user Commodore64user Feb 12, 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.

You may investigate what have changed and how textboxwidget vs. htmlboxwidget differ

I think I am going to be busy that day ;) but a quick blame yields bf35875

and this golden nugget sums it up really #12948 (review)

Comment thread frontend/ui/widget/dictquicklookup.lua Outdated
Comment on lines 1827 to 1831
if self.is_html then
-- For dictionary content, highlight_text should contain the complete text selection
selected_text = selection_widget.highlight_text
else
-- For wiki content, extract the selected text using the indices

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.

Please test with a text dict, as your comments feel wrong.

@Commodore64user Commodore64user Feb 12, 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.

could you kindly turn the Wiktionary into one? I didn't really understand what you said in the other comment... ;) or better yet, if you could test it, that'd be great. Please

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.

I won't test it for you - you need to see it to update your comment.
Take your favorite dict, copy the blah.ifo file (present in its directory on your device) to blah.ifo.ORIG, edit the blah.ifo, change in it:
sametypesequence=h
to
sametypesequence=x.
After this, when showing results, you should have plain text (and no longer italic or bold or various font sizes), like with Wikipedia results.

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.

is this what you mean For non-html content, extract the selected text using the indices?

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.

With TextBoxWidget, extract the selected text using the indices
With HtmlBoxWidget, we can blah blah
But I'd rather wait for @TnS-hun thoughts before doing too much kitchening in here.

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.

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.

So, do you confirm you have the same behaviour on this text dict as with Wikipedia results?

@Commodore64user Commodore64user Feb 12, 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.

yes, it worked just fine.

edit: I am not sure if it still considered self.is_html or not, but I was able to get definitions either way

Comment thread frontend/ui/widget/dictquicklookup.lua Outdated
self.image_alt_face = Font:getFace("cfont", font_size_alt)
self.allow_text_selection = Device:hasScreenKB() or Device:hasKeyboard()
if self.allow_text_selection then
self.allow_nt_text_selection = Device:hasScreenKB() or Device:hasKeyboard()

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.

If it's not not IsTouchDevice(), don't name it "nt_text_selection"...
Name it "key_text_selection".

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.

Roger that

Comment thread frontend/ui/widget/dictquicklookup.lua Outdated
highlight_text_selection = true,
post_paintTo_func = function(widget, bb, x, y)
-- We should override the widget's paintTo method to draw our indicator
paintTo = function(widget, bb, x, y)

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.

paintTo = self.allow_by_key_text_selection and function(widget, bb, x, y) ...

@poire-z

poire-z commented Feb 13, 2025

Copy link
Copy Markdown
Contributor

could this have anything to do with it @poire-z, I have a hunch this is a CRengine thing.

I don't think so, there's no crengine involved here.
What you paster is how we pass CSS to MuPDF for html results rendering (where you don't have a problem if I get you right) - nothing to do with crengine (where we often dicuss CSS - but unrelated to your issue).
No CSS with text, the justification is done by code in textboxwidget.

Have you tested how it works (wikipedia, text results) on a touch device, if you notice the same issue ? If yes, that's my problem, show me more.
If not, it's yours problem with this PR :) not really triggering the same gesture/events/coordinates as on touch devices.

@Commodore64user

Commodore64user commented Feb 13, 2025

Copy link
Copy Markdown
Member Author

Okay, let me have another look. In the meantime i'll let you go back to Page Browser and Book Map ;)

Edit: I'm sick of eating pears

@Commodore64user

Commodore64user commented Feb 13, 2025

Copy link
Copy Markdown
Member Author

@poire-z I can replicate the issue on kindle voyage as well, so it is a you problem ;) (my favourite kind).

you can test it yourself, search for the word approach on the wiktionary EN-EN no-etymology (as seen on the original screenshots #13232 (comment)), you can emulate it at 600x800, text needs to be justified and default font size (20). Try to select the word tennis and you always get intransitive

@poire-z

poire-z commented Feb 13, 2025

Copy link
Copy Markdown
Contributor

I won't be able to replicate, too many variables.
Show me a screenshot of where that word is, mark the location of your cross, and what ends up highlighted.
Unlike on the screenshot you mention, you shouldn't get any italic if it's a text-dict result.
image

Do you have italic or not ? If yes, it's a html-dict and a @TnS-hun problem :). If not, it's still mine :/
But you did witness it on html dict if we trust that screenshot ? We need some clarifying here.

@Commodore64user

Commodore64user commented Feb 13, 2025

Copy link
Copy Markdown
Member Author

I won't be able to replicate, too many variables.

have you even tried though? I gave you all the relevant ones, but again; 600x800 screen, wiktionary (no-etymology version), text justified and font size 20.

It is an is_html issue, or at least I haven't been able to find a wiki that fails.

The screenshots are accurate, both crosshairs and fingers placed on tennis (technically any word on that line really), incorrectly select intransitive.

edit: @poire-z it was incredible easy for me to replicate the dict lookup on the emulator. seriously

@poire-z

poire-z commented Feb 14, 2025

Copy link
Copy Markdown
Contributor

I understood you had the problem with text results :/ wrongly apparently.
So, if all works fine with text dicts, I'm fine.
And I don't need to test and reproduce anything.
I'm not familiar with all the html dicts and MuPDF interactions, it's possible with text justifications we don't account for the spacing added by MuPDF between each word box if we only account for their lengths, dunno. Anyway, that's more for @TnS-hun to investigate, easier it happens with touch too.

@Commodore64user

Copy link
Copy Markdown
Member Author

to make it, even clearer. here it is happening on the emulator.

Screen.Recording.2025-02-14.at.03.10.13.mov

@TnS-hun

TnS-hun commented Feb 14, 2025

Copy link
Copy Markdown
Contributor

Have you tried to reproduce the problem in the emulator with mouse without your changes? I can only look at tomorrow.

I could reproduce it by changing the font to size 20.

Looks like page:getPageText() returns multiple lines for the same line (notice the same y values) and getWordIndices uses the first one:
tennis-problem

@poire-z

poire-z commented Feb 14, 2025

Copy link
Copy Markdown
Contributor

Looks like page:getPageText() returns multiple lines for the same line (notice the same y values) and getWordIndices uses the first one:

(Just a guess: MuPDF might be returning multiple rects - or table of rects - on the same line, when there are multiple wrapping HTML nodes on that line, a new rect/table when we get in/out of each node ?)

TnS-hun added a commit to TnS-hun/koreader that referenced this pull request Feb 17, 2025
In same cases MuPDF returns a visually single line of text as multiple lines. Merge such lines to ensure that getSelectedText works properly.

See: koreader#13232 (comment)
@TnS-hun

TnS-hun commented Feb 17, 2025

Copy link
Copy Markdown
Contributor

I made a simple fix for the issue: #13276

Frenzie pushed a commit that referenced this pull request Feb 17, 2025
In same cases MuPDF returns a visually single line of text as multiple lines. Merge such lines to ensure that getSelectedText works properly.

See: #13232 (comment)
self.html_dictionary_link_tapped_callback(self.dictionary, link)
end,
-- We need to override the widget's paintTo method to draw our indicator
paintTo = self.allow_key_text_selection and function(widget, bb, x, y)

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.

Have you tested all this on a device without keys ?
Feels like paintTo = false and function will make paintTo be false and still ovveride the original method with false and may crash.
May be best to add a or nil at the end

@Commodore64user Commodore64user Feb 17, 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.

no i haven't. only tested on emu and k4. will add the or nil then

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.

follow up question. how can I make the emulator be touch only? or useDPadAsActionKeys?

currently doing ./kodev run -t somewhat replicates non-touch but still ignores useDPAd though

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.

Dunno if there are options. When I need that, I just hack into frontend/device/sdl/device.lua to change the yes/no I want to have.

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.

By editing device/sdl.

@poire-z poire-z left a comment

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 fond of having DictQuickLookup grow 10% more fat, and adhoc menu items because you don't want the same cursor speed in reader vs dict - all this for obsolete non-touch devices where everything is unpractical....
But well, we don't choose our family.

@Frenzie Frenzie added this to the 2025.02 milestone Feb 17, 2025
@Frenzie Frenzie added the NT Non Touch devices label Feb 17, 2025
Comment thread frontend/ui/widget/dictquicklookup.lua
@Frenzie Frenzie merged commit 546cb14 into koreader:master Feb 17, 2025
@Commodore64user Commodore64user deleted the dict-text-select branch February 17, 2025 23:08
0xstillb pushed a commit to 0xstillb/koreader-thai that referenced this pull request May 9, 2026
In same cases MuPDF returns a visually single line of text as multiple lines. Merge such lines to ensure that getSelectedText works properly.

See: koreader#13232 (comment)
0xstillb pushed a commit to 0xstillb/koreader-thai that referenced this pull request May 9, 2026
…reader#13232)

Initially, when you open the dictionary widget from the reader, FocusManager will function as usual. On Kindle 4 or devices with a keyboard, I have assigned the shortcuts <kbd>screenkb</kbd> + <kbd>up</kbd>/<kbd>down</kbd>   or <kbd>shift</kbd> + <kbd>up</kbd>/<kbd>down</kbd> to initiate text selection (as a remainder, on kindle text selection in reader is initiated with <kbd>up</kbd>/<kbd>down</kbd> so it is fairly similar). At this point, FocusManager is disabled, allowing the cursor keys (and press) to control the now-visible crosshairs. Pressing <kbd>back</kbd> should stop text selection and restore FocusManager’s control of the widget.
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 this pull request may close these issues.

4 participants