Skip to content

VocabBuilder: Fix and improve non-touch device experience#12030

Merged
Frenzie merged 9 commits into
koreader:masterfrom
weijiuqiao:vocabbuilder_dev
Jun 16, 2024
Merged

VocabBuilder: Fix and improve non-touch device experience#12030
Frenzie merged 9 commits into
koreader:masterfrom
weijiuqiao:vocabbuilder_dev

Conversation

@weijiuqiao

@weijiuqiao weijiuqiao commented Jun 15, 2024

Copy link
Copy Markdown
Contributor

It seems due to the FocusManager setsDirty self.show_parent or self when it focuses on a target item, the target item does not show focus when the manager has a show_parent.

Fix #12026


This change is Reviewable

@Frenzie Frenzie added this to the 2024.06 milestone Jun 15, 2024
Comment thread plugins/vocabbuilder.koplugin/main.lua Outdated
@Commodore64user

Commodore64user commented Jun 15, 2024

Copy link
Copy Markdown
Member

testing this, works fine now. Only one minor thing, for consistency, the menu button should both open and close the hamburger menu, right now it can only be opened by it but not closed (and yes I know you can do it with the back button).

btw thanks! for fixing it so quickly

@weijiuqiao

Copy link
Copy Markdown
Contributor Author

the menu button should both open and close the hamburger menu

It's done.

@Commodore64user

Copy link
Copy Markdown
Member

that last commit is not strictly necessary (at least on kindle) as there is support for long press there already but I am not really bothered by it either, I was in fact going to say that pressing "Menu" inside that widget was crashing the programme but that technically solves that so... one more thing I just discovered is that the "search" widget does not support back button presses either, you can technically exit it by using focus and selecting cancel but for consistency again, best to map it to "Back" button keys.

Comment thread plugins/vocabbuilder.koplugin/main.lua Outdated
self.layout = {}
if Device:hasKeys() then
self.key_events.Close = { { Device.input.group.Back } }
self.key_events.Close = { { { "Back", "Menu" } } }

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.

@Frenzie bringing your attention here as this might affect other non-touch devices...

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.

Yes, we would want to maintain the full Back group.

In ButtonDialog (which is used in most places) we did this:

        if Device:hasKeys() then
            local back_group = util.tableDeepCopy(Device.input.group.Back)
            if Device:hasFewKeys() then
                table.insert(back_group, "Left")
                self.key_events.Close = { { back_group } }
            else
                table.insert(back_group, "Menu")
                self.key_events.Close = { { back_group } }
            end

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed accordingly.

@Commodore64user

Copy link
Copy Markdown
Member

local util = require("util") is missing

@weijiuqiao

Copy link
Copy Markdown
Contributor Author

Oops. Thanks for helping testing.

Comment thread plugins/vocabbuilder.koplugin/main.lua Outdated
local TitleBar = require("ui/widget/titlebar")
local ToggleSwitch = require("ui/widget/toggleswitch")
local UIManager = require("ui/uimanager")
local util = require("util")

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.

Could you move it under WidgetContainer above _, see https://github.com/koreader/koreader-base/wiki/Coding-style#order-of-requires

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@Frenzie Frenzie added the NT Non Touch devices label Jun 16, 2024
@Frenzie Frenzie added the Plugin label Jun 16, 2024
@Frenzie Frenzie merged commit e19bed3 into koreader:master Jun 16, 2024
Commodore64user pushed a commit to Commodore64user/KOReader_fork that referenced this pull request Aug 2, 2024
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

NT Non Touch devices Plugin

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Vocabulary builder won't show focus on NT

4 participants