Skip to content

[ReaderHighlight] NT: add better support for extending highlights#13917

Merged
Frenzie merged 3 commits into
koreader:masterfrom
Commodore64user:extend-highlight
Jun 21, 2025
Merged

[ReaderHighlight] NT: add better support for extending highlights#13917
Frenzie merged 3 commits into
koreader:masterfrom
Commodore64user:extend-highlight

Conversation

@Commodore64user

@Commodore64user Commodore64user commented Jun 6, 2025

Copy link
Copy Markdown
Member

what's new

related PRs

screen recording

name

This change is Reviewable

@Frenzie Frenzie left a comment

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.

Comment thread frontend/apps/reader/modules/readerhighlight.lua
@poire-z

poire-z commented Jun 9, 2025

Copy link
Copy Markdown
Contributor

Not familiar with select_mode, @hius07 is probably most apt to review this.

@Commodore64user

Copy link
Copy Markdown
Member Author

Any comments?

@Frenzie Frenzie added this to the 2025.06 milestone Jun 21, 2025
@Frenzie Frenzie merged commit d993146 into koreader:master Jun 21, 2025
2 of 4 checks passed
this:startSelection(index)
this:onClose()
if not Device:isTouchDevice() then
self:onStartHighlightIndicator()

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.

self or this?

return true
end
-- Check if we're in select mode (or extending an existing highlight)
if self.select_mode and self.highlight_idx then

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.

highlight_idx always exists in the select mode.

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 don't understand NT things, is onHighlightPress a combination of tap and hold?
On touch devices the action onTap in select mode depends on whether we tap a starting fragment or not:

if self.select_mode then
if box.index == self.highlight_idx then
-- tap on the first fragment: abort select mode, clear highlight

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.

NT essentially fakes all touch gestures, depending on context onHighlightPress is either a tap, or a hold or a hold release. In this case following the exact touch device behaviour was cumbersome with buttons, so i made some adjustments, tapping the first fragment is now replaced by pressing back (makes more sense) and the double step selection also didn’t make much sense, see the screen recording at the top

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.

Okay

@Commodore64user Commodore64user deleted the extend-highlight branch June 21, 2025 11:06
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