Skip to content

[Common settings] remove screenshot setting from unsupported devices#12293

Closed
Commodore64user wants to merge 1 commit into
koreader:masterfrom
Commodore64user:remove-settings
Closed

[Common settings] remove screenshot setting from unsupported devices#12293
Commodore64user wants to merge 1 commit into
koreader:masterfrom
Commodore64user:remove-settings

Conversation

@Commodore64user

@Commodore64user Commodore64user commented Aug 5, 2024

Copy link
Copy Markdown
Member

removes screenshot folder setting from unsupported devices (i.e non-touch bar kindle)


This change is Reviewable

@hius07

hius07 commented Aug 5, 2024

Copy link
Copy Markdown
Member

if not Device:isTouchDevice() then return end

@Frenzie

Frenzie commented Aug 5, 2024

Copy link
Copy Markdown
Member

While this setting is of no import, they can take screenshots perfectly fine through profiles.

@NiLuJe

NiLuJe commented Aug 5, 2024

Copy link
Copy Markdown
Member

Didn't we also keep the stupid-ass default Kindle shortcut for screenshots? (Which I can't even remember given how stupid it is. It's like Sym + something + something).

@Frenzie

Frenzie commented Aug 5, 2024

Copy link
Copy Markdown
Member

That's covered here. The exclusion considers it. But in any case, I'd rather just keep the setting.

if Device:hasKeyboard() then
self.key_events.KeyPressShoot = {
{ "Alt", "Shift", "G" }, -- same as stock Kindle firmware
event = "KeyPressShoot",
}
elseif Device:hasScreenKB() then
-- kindle 4 case: same as stock firmware.
self.key_events.KeyPressShoot = {
{ "ScreenKB", "Menu" },
event = "KeyPressShoot",
}
-- unable to add other non-touch devices as simultaneous key presses won't work without modifiers
end

I think profiles doesn't have the ability to run an action in x seconds.

@Commodore64user

Copy link
Copy Markdown
Member Author

But in any case, I'd rather just keep the setting.

Oh well, this makes absolutely no difference to me. I just thought it would be better not to give false hope to hasFewKeys when even with the Profile trick, is not really "perfectly fine" to capture them. At least in my attempts i could not hide the Menu which meant all screenshots are half the profile menu (to trigger said profile) and half whatever you want to screen shot.

Didn't we also keep the stupid-ass default Kindle shortcut for screenshots?

There is actually a reason for that. If you google duckduckgo "how to take screenshots on kindle keyboard", every result will tell you to do it that way. So why not build on top of it?

@Frenzie

Frenzie commented Aug 5, 2024

Copy link
Copy Markdown
Member

At least in my attempts i could not hide the Menu which meant all screenshots are half the profile menu (to trigger said profile) and half whatever you want to screen shot.

That's also covered by what I said about the timeout, although that was more about capturing different screens.

You could update the PR like this (or close this one and start a new one):

diff --git a/plugins/profiles.koplugin/main.lua b/plugins/profiles.koplugin/main.lua
index 768518f97..ca356f742 100644
--- a/plugins/profiles.koplugin/main.lua
+++ b/plugins/profiles.koplugin/main.lua
@@ -124,14 +124,14 @@ function Profiles:getSubMenuItems()
                 text = _("Execute"),
                 callback = function(touchmenu_instance)
                     touchmenu_instance:onClose()
-                    self:onProfileExecute(k, { qm_show = false })
+                    UIManager:nextTick(self.onProfileExecute, self, k, { qm_show = false })
                 end,
             },
             {
                 text = _("Show as QuickMenu"),
                 callback = function(touchmenu_instance)
                     touchmenu_instance:onClose()
-                    self:onProfileExecute(k, { qm_show = true })
+                    UIManager:nextTick(self.onProfileExecute, self, k, { qm_show = true })
                 end,
             },
             {

Frenzie added a commit to Frenzie/koreader that referenced this pull request Aug 5, 2024
Mainly a difference without distinction, but in some edge cases like screenshots it keeps the menu on the screen.
See <koreader#12293 (comment)>.

The "Show as QuickMenu" is really just for balance; it makes little to no difference.
@Frenzie

Frenzie commented Aug 5, 2024

Copy link
Copy Markdown
Member

I took care of it since I was already looking at it.

@Frenzie Frenzie closed this Aug 5, 2024
@Commodore64user Commodore64user deleted the remove-settings branch August 5, 2024 20:23
Frenzie added a commit that referenced this pull request Aug 6, 2024
…12299)

Mainly a difference without distinction, but in some edge cases like screenshots it keeps the menu on the screen.
See <#12293 (comment)>.

The "Show as QuickMenu" is really just for balance; it makes little to no difference.
@Commodore64user Commodore64user restored the remove-settings branch February 6, 2025 23:25
@Commodore64user

Copy link
Copy Markdown
Member Author

since the other solution didn't work, would you reconsider this now @Frenzie ?

@Frenzie

Frenzie commented Feb 7, 2025

Copy link
Copy Markdown
Member

Okay.

@Commodore64user

Copy link
Copy Markdown
Member Author

You can reopen it then, (i can't since you closed it). It's rebased and all ;)

@Frenzie Frenzie modified the milestone: 2025.02 Feb 7, 2025
@Frenzie

Frenzie commented Feb 7, 2025

Copy link
Copy Markdown
Member

I can't. It says the branch was "force-pushed or recreated". Please open a new PR.

0xstillb pushed a commit to 0xstillb/koreader-thai that referenced this pull request May 9, 2026
…oreader#12299)

Mainly a difference without distinction, but in some edge cases like screenshots it keeps the menu on the screen.
See <koreader#12293 (comment)>.

The "Show as QuickMenu" is really just for balance; it makes little to no difference.
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