-
-
Notifications
You must be signed in to change notification settings - Fork 669
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
Setting the font of a toga.Selection (Android and Winforms) #1393
Conversation
@freakboy3742 Please review this PR, thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not wild about the approach that you've taken here. There's a lot of logic tied up in the Java side of things; this means (a) the template is increasingly Toga specific, and (b) you need to know Java (and must be able to fix a Java template) to be able to address any bug, or add any feature.
I know there are some existing shims on the existing template; however, those are either directly related to the bootstrapping process, or are the barest possible shim that can exist in Java to be extended by Python.
My long term goal is to add some metaprogramming to the templating process so that it is possible to define the existence of these shims in Python (or, at least, as part of some sort of project configuration), and have the relevant Java generated as part of the Briefcase templating process. To that end, adding complex logic on the Java side is somewhat counterproductive.
from .base import Widget, align | ||
|
||
BeewareSpinnerAdapter = JavaClass("org/beeware/android/BeewareSpinnerAdapter") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need to replicate "Beeware" in this class name; org.beeware.android.SpinnerAdapter
is sufficiently unique.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've done that because there already is a SpinnerAdapter
in Android - and it is not a class, but an Interface.
Of course, technically, it would still work because we are in a different namespace, but I thought it would be less confusing if I use a new name.
How about CustomSpinnerAdapter or ExtendedSpinnerAdapter ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The template is not really Toga specific.
The new Class could be used by any GUI. It just provides the necessary features to control the (size of the) font - which, in my opintion, is a short-coming of the standard implementation in Android.
This is currently the only approach I know to be working. If rubicon would allow sub-classing, then we might do it all in Python code. |
This is one of those cases where we could take the quick route, but that will introduce technical debt that we will need to live with over time. I'm indicating that I'm uncomfortable with the level of technical debt that the quick solution will introduce, and that the acceptable solution here starts with a change in Rubicon (and possibly Briefcase). |
Closing on the basis that this specific implementation isn't headed in the right direction. There's clearly a bigger problem that we need to resolve; I want to address that underlying problem, rather than commit resources to managing a hack to solve this specific instance of the underlying problem. |
This PR allows us to set the font of a toga.Selection on Android and Winforms
The Android implementation requires the template from this PR:
beeware/briefcase-android-gradle-template#40
PR Checklist: