ReaderHighlight: adjustable highlight dialog position #11116
Conversation
| -- NOTE: Disable merging for this update, | ||
| -- or the buggy Sage kernel may alpha-blend it into the page (with a bogus alpha value, to boot)... | ||
| UIManager:show(self.highlight_dialog, "[ui]") |
There was a problem hiding this comment.
This comment still feels relevant (explaining why the [ ] around [ui]).
| local screen_h = self.screen_h - Size.padding.small -- do not stick to the edge | ||
| local anchor_y, prefers_pop_down | ||
| if position == "top" then | ||
| anchor_y = Size.padding.small | ||
| prefers_pop_down = true | ||
| elseif position == "bottom" then | ||
| anchor_y = screen_h |
There was a problem hiding this comment.
Lying about screen_h :) while you could probably do in the bottom branch like you do for the top branch: anchor_y = screen_h - Size.padding.small.
(I see that you reuse this hacked screen_h below - but the unbalance in this 2 branches reads odd. And no substraction computation saved, on the contrary it's avoided if "top", if you do it explicitely in the "bottom" and "gesture" branches.)
| else -- above box with gest_pos | ||
| anchor_y = text_box.y - Size.padding.small |
There was a problem hiding this comment.
Shouldn't there be a - text_box.h here so it is fully above text_box.y ?
There was a problem hiding this comment.
koreader/frontend/ui/widget/container/movablecontainer.lua
Lines 139 to 140 in 88d6613
| {_("Top"), "top"}, | ||
| {_("Center"), "center"}, | ||
| {_("Bottom"), "bottom"}, | ||
| {_("Gesture"), "gesture"}, |
There was a problem hiding this comment.
"Gesture" standalone feels a little bit odd. "Gesture position", "Touch position", "Near touch" ? @Frenzie ?
There was a problem hiding this comment.
Gesture position, I think
koreader/plugins/gestures.koplugin/main.lua
Line 248 in 88d6613
| local position = G_reader_settings:readSetting("highlight_dialog_position", "center") | ||
| for __, v in ipairs(highlight_dialog_position) do | ||
| if v[2] == position then | ||
| return T(_("Highlight dialog position: %1"), v[1]:lower()) |
There was a problem hiding this comment.
Also, "dialog" feels like some technical / internal terminology.
"Highlight popup position" ?
There was a problem hiding this comment.
|
I come here to say this is a great feature and it works charms with the option set to “gesture”! Thank you very much @hius07 ! This also works with dictionary lookup window (chosen as a single-word highlighting action). When this feature is set to “center”, highlight window (if more than one word is selected) shows correctly in the center. However, the dictionary popup still shows up as if it was “gesture”. But if I then tap another word in this dictionary window, a second dictionary windows shows up - but this time around in the center. So it looks a bit inconsistent. There is no inconsistency issue when suing “gesture”: second dictionary window shows up at the same location as the first one. I am using the “gesture” option all the time, so this is not an issue for me. Just sharing my observations :) |
|
Thank you for the feedback.
Not related to the highlight dialog, Dictionary window does not use this setting, |
|
Interesting, it must be working like this before this setting for highlights was introduced. Dictionary window shows up right above or right below the highlighted word in most cases except when words are right in the middle - then screen height doesn't allow it to be neither above/below the word so the windows shows up just in the center. I didn't know about all these on Onyx Boox, because I was using external dictionary app, so I don't know when the dictionary window behavior changed exactly :D |
Default to "center". Closes #11081.
This change is