Skip to content

Highlights: quicker Style and Note marker dialogs#13223

Merged
hius07 merged 10 commits into
koreader:masterfrom
hius07:hl-style
Feb 12, 2025
Merged

Highlights: quicker Style and Note marker dialogs#13223
hius07 merged 10 commits into
koreader:masterfrom
hius07:hl-style

Conversation

@hius07
Copy link
Copy Markdown
Member

@hius07 hius07 commented Feb 8, 2025

Immediate effect, saving two taps as discussed in #13187.

1


This change is Reviewable

@poire-z
Copy link
Copy Markdown
Contributor

poire-z commented Feb 8, 2025

If I tap on a previously made highlight, I will get that popup directly instead of the one with arrows to extend the highlight ?
Or do I still get the one with arrows, and only after taping one of its button I get this popup ?

@hius07
Copy link
Copy Markdown
Member Author

hius07 commented Feb 8, 2025

Or do I still get the one with arrows, and only after taping one of its button I get this popup ?

This.

@hius07
Copy link
Copy Markdown
Member Author

hius07 commented Feb 8, 2025

RadioButton widget required pressing Apply and Close buttons.
It is changed to ButtonDialog with immediate effect.

@hius07
Copy link
Copy Markdown
Member Author

hius07 commented Feb 8, 2025

It would be good to change the Color dialog in the same way, but we do not have colored buttons.

@Galunid
Copy link
Copy Markdown
Member

Galunid commented Feb 8, 2025

I'm not sure if making this a new default is a good idea. While I really like the new menu compared to the old radio buttons one, I definitely prefer "generic" menu as a default tap-on-highlight action, since I don't change highlight style at all, but often use buttons to adjust the highlighted text. This mostly comes down to preference. I don't think it warrants change of default behavior, but that's debatable.

I'd propose adding new action in Highlights menu "Ask"/"Customize". It should show this popup when the user finishes text selection. Customize could show both Style/Color adjustment.

In that case tap-on-highlight could still show the "generic" menu and user would still have the ability to customize highlights as soon as they are made.

This is meant more as a food for thought rather than critique of the PR. I'm not arguing whether it should/shouldn't be merged.

@hius07
Copy link
Copy Markdown
Member Author

hius07 commented Feb 8, 2025

Thanks for the ideas.
No changes to the 'generic' Highlight menu. This new menu just replaces radio-buttons widget that is called by pressing the Style button in the original Highlight menu.

@hius07
Copy link
Copy Markdown
Member Author

hius07 commented Feb 9, 2025

The same for Note marker dialog:

2

@hius07 hius07 changed the title Highlights: quicker style dialog Highlights: quicker Style and Note marker dialogs Feb 9, 2025
function ReaderHighlight:onShowHighlightDialog(index)
function ReaderHighlight:showHighlightDialog(index)
local item = self.ui.annotation.annotations[index]
local edit_highlight_dialog
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.

Oh, nice :). That instance variable was indeed awkward!

@hius07 hius07 merged commit 70fcc90 into koreader:master Feb 12, 2025
@hius07 hius07 deleted the hl-style branch February 12, 2025 07:14
@hius07 hius07 added this to the 2025.02 milestone Feb 12, 2025
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