Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A few graphics fixes after #4541 #4554

Merged
merged 16 commits into from Feb 7, 2019

Conversation

Projects
None yet
3 participants
@NiLuJe
Copy link
Member

NiLuJe commented Feb 7, 2019

  • Various FocusManager related tweaks to limit its usage to devices with a DPad, and prevent initial button highlights in Dialogs on devices where it makes no sense (i.e., those without a DPad. And even on DPad devices, I'm not even sure how we'd go about making one of those pop up anyway, because no Touch ;)!).
  • One mysterious fix to text-only Buttons so that the flash_ui highlight always works, and always honors FrameContainer's pill shape. (Before that, an unhighlight on a text button with a callback that didn't repaint anything [say, the find first/find last buttons in the Reader's search bar when you're already on the first/last match] would do a square black highlight, and a white pill-shaped unhighlight (leaving the black corners visible)).
    The workaround makes absolutely no sense to me (as self[1] -> self.frame, AFAICT), but it works, and ensures all highlights/unhighlights are pill-shaped, so at least we're not doing maths for rounded corners for nothing ;).

NiLuJe added some commits Feb 7, 2019

Limit key event setup to actually abailable keys
(... Mostly. hasKeys is generally used for PageTurn buttons, but it also
includes device-specific Buttons (Home, or the Kindle ones, f.g.).
Simplify that check
Every device w/ hasKeyboard also hasKeys by design ;).
Simplify
Same as scrolltextwidget
This is insane, but it works :?
Now the highlight is *always* pill-shaped.

My brain hurts, as self[1] -> self.frame :?
Eh, at least limit that insane workaround to where it's needed...
(Text buttons, icons are fine, AFAICT...).
@poire-z

This comment has been minimized.

Copy link
Contributor

poire-z commented Feb 7, 2019

The workaround makes absolutely no sense to me (as self[1] -> self.frame, AFAICT), but it works

Indeed :|
What if you use only self.frame ? What if you use twice self.frame ? or twice self[1] ?

@@ -813,6 +813,9 @@ function Menu:init()
self.key_events.PrevPage = {
{Input.group.PgBack}, doc = "goto previous page of the menu"
}
end

if Device:hasDPad() then

This comment has been minimized.

@Frenzie

Frenzie Feb 7, 2019

Member

Doesn't this affect regular computer keyboards too? (AppImage)

This comment has been minimized.

@Frenzie

Frenzie Feb 7, 2019

Member

Oh never mind, the SDL device already hasDPad = yes.

This comment has been minimized.

@NiLuJe

NiLuJe Feb 7, 2019

Author Member

Yeah, I double-checked that stuff with a keyboard always had hasDPad enabled ;).

@NiLuJe

This comment has been minimized.

Copy link
Member Author

NiLuJe commented Feb 7, 2019

@poire-z : I would hope that it'd behave exactly the same ^^.

I'll try it, for shit'n giggles ;).

@NiLuJe

This comment has been minimized.

Copy link
Member Author

NiLuJe commented Feb 7, 2019

@poire-z : Yes, any combination works, the key aspect is just needing two repaint passes. o_O.

Would you prefer me to stick to a single nomenclature to make it more obvious, or is the comment enough?

@poire-z

This comment has been minimized.

Copy link
Contributor

poire-z commented Feb 7, 2019

Would you prefer me to stick to a single nomenclature to make it more obvious

I think it would indeed be clearer to just use one of self.frame or self[1], and just mention that a double repaint is needed for some unknown reason :)

@NiLuJe

This comment has been minimized.

Copy link
Member Author

NiLuJe commented Feb 7, 2019

Okay, went with self[1] as that's what most other flash_ui enabled widgets do ;).

(Although, IIRC, there's a few sneaky cases where [1] isn't frame... :D).

@NiLuJe NiLuJe merged commit 8189945 into koreader:master Feb 7, 2019

1 check passed

ci/circleci Your tests passed on CircleCI!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.