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.

Sign up for GitHub

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

Jump to bottom

Correct scaling of MIN_WIDTH and MIN_HEIGHT on Android and WinForms #2186

Merged
merged 2 commits into from
Nov 2, 2023
Merged

Correct scaling of MIN_WIDTH and MIN_HEIGHT on Android and WinForms #2186

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4aa24c4
Android: correct scaling of MIN_WIDTH and MIN_HEIGHT
mhsmith Nov 2, 2023
bf47925
WinForms: correct scaling of MIN_WIDTH and MIN_HEIGHT
mhsmith Nov 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 5 additions & 10 deletions android/src/toga_android/widgets/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from abc import ABC, abstractmethod
from decimal import ROUND_HALF_EVEN, ROUND_UP, Decimal
from decimal import ROUND_HALF_EVEN, Decimal

from android.graphics import PorterDuff, PorterDuffColorFilter, Rect
from android.graphics.drawable import ColorDrawable, InsetDrawable
Expand Down Expand Up @@ -182,18 +182,13 @@ def remove_child(self, child):
# TODO: consider calling requestLayout or forceLayout here
# (https://github.com/beeware/toga/issues/1289#issuecomment-1453096034)
def refresh(self):
intrinsic = self.interface.intrinsic
intrinsic.width = intrinsic.height = None
# Default values; may be overwritten by rehint().
self.interface.intrinsic.width = at_least(self.interface._MIN_WIDTH)
self.interface.intrinsic.height = at_least(self.interface._MIN_HEIGHT)
Copy link
Member

Choose a reason for hiding this comment

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

Should these default values be scaled out?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, because they're already in CSS pixels. The intention of this PR is that when sizes are assigned to intrinsic.width or intrinsic.height, they are already in CSS pixels as the interface layer expects, so they don't need to be scaled again.

self.rehint()
assert intrinsic.width is not None, self
assert intrinsic.height is not None, self

intrinsic.width = self.scale_out(intrinsic.width, ROUND_UP)
intrinsic.height = self.scale_out(intrinsic.height, ROUND_UP)

@abstractmethod
def rehint(self):
...
pass


def align(value):
Expand Down
5 changes: 0 additions & 5 deletions android/src/toga_android/widgets/box.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
from android.widget import RelativeLayout
from travertino.size import at_least

from .base import Widget

Expand All @@ -10,7 +9,3 @@ def create(self):

def set_background_color(self, value):
self.set_background_simple(value)

def rehint(self):
self.interface.intrinsic.width = at_least(0)
self.interface.intrinsic.height = at_least(0)
10 changes: 8 additions & 2 deletions android/src/toga_android/widgets/button.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from decimal import ROUND_UP

from android.view import View
from android.widget import Button as A_Button
from java import dynamic_proxy
Expand Down Expand Up @@ -40,5 +42,9 @@ def rehint(self):
View.MeasureSpec.UNSPECIFIED,
View.MeasureSpec.UNSPECIFIED,
)
self.interface.intrinsic.width = at_least(self.native.getMeasuredWidth())
self.interface.intrinsic.height = self.native.getMeasuredHeight()
self.interface.intrinsic.width = self.scale_out(
at_least(self.native.getMeasuredWidth()), ROUND_UP
)
self.interface.intrinsic.height = self.scale_out(
self.native.getMeasuredHeight(), ROUND_UP
)
5 changes: 0 additions & 5 deletions android/src/toga_android/widgets/canvas.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
from java import dynamic_proxy, jint
from java.io import ByteArrayOutputStream
from org.beeware.android import DrawHandlerView, IDrawHandler
from travertino.size import at_least

from toga.widgets.canvas import Baseline, FillRule

Expand Down Expand Up @@ -265,7 +264,3 @@ def get_image_data(self):

def set_background_color(self, value):
self.set_background_simple(value)

def rehint(self):
self.interface.intrinsic.width = at_least(0)
self.interface.intrinsic.height = at_least(0)
5 changes: 0 additions & 5 deletions android/src/toga_android/widgets/detailedlist.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
from android.widget import ImageView, LinearLayout, RelativeLayout, ScrollView, TextView
from androidx.swiperefreshlayout.widget import SwipeRefreshLayout
from java import dynamic_proxy
from travertino.size import at_least

from .base import Widget

Expand Down Expand Up @@ -243,7 +242,3 @@ def scroll_to_row(self, row):
hit_rect,
True, # Immediate, not animated
)

def rehint(self):
self.interface.intrinsic.width = at_least(self.interface._MIN_WIDTH)
self.interface.intrinsic.height = at_least(self.interface._MIN_HEIGHT)
8 changes: 5 additions & 3 deletions android/src/toga_android/widgets/imageview.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from decimal import ROUND_UP

from android.widget import ImageView as A_ImageView

from toga.widgets.imageview import rehint_imageview
Expand All @@ -21,7 +23,7 @@ def set_image(self, image):

def rehint(self):
# User specified sizes are in "pixels", which is DP;
# we need to convert all sizes into SP.
# we need to convert all sizes into physical pixels.
dpi = self.native.getContext().getResources().getDisplayMetrics().densityDpi
# Toga needs to know how the current DPI compares to the platform default,
# which is 160: https://developer.android.com/training/multiscreen/screendensities
Expand All @@ -30,8 +32,8 @@ def rehint(self):
width, height, aspect_ratio = rehint_imageview(
image=self.interface.image, style=self.interface.style, scale=scale
)
self.interface.intrinsic.width = width
self.interface.intrinsic.height = height
self.interface.intrinsic.width = self.scale_out(width, ROUND_UP)
self.interface.intrinsic.height = self.scale_out(height, ROUND_UP)
if aspect_ratio is not None:
self.native.setScaleType(A_ImageView.ScaleType.FIT_CENTER)
else:
Expand Down
5 changes: 4 additions & 1 deletion android/src/toga_android/widgets/internal/pickers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from abc import ABC, abstractmethod
from decimal import ROUND_UP

from android.view import View
from android.widget import EditText
Expand Down Expand Up @@ -42,4 +43,6 @@ def create(self):
def rehint(self):
self.interface.intrinsic.width = at_least(300)
Copy link
Member

Choose a reason for hiding this comment

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

Should this constant value be scaled (and/or expressed in terms that are suitable for scaling?)

Copy link
Member Author

Choose a reason for hiding this comment

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

No, for the same reason as above. And since date and time pickers look quite different on each platform, I think it makes sense for the backend to have its own value in this case, rather than defining a _MIN_WIDTH that they all share.

self.native.measure(View.MeasureSpec.UNSPECIFIED, View.MeasureSpec.UNSPECIFIED)
self.interface.intrinsic.height = self.native.getMeasuredHeight()
self.interface.intrinsic.height = self.scale_out(
self.native.getMeasuredHeight(), ROUND_UP
)
8 changes: 6 additions & 2 deletions android/src/toga_android/widgets/label.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from decimal import ROUND_UP

from android.os import Build
from android.text import Layout
from android.util import TypedValue
Expand Down Expand Up @@ -68,13 +70,15 @@ def rehint(self):
# This is the height with word-wrapping disabled.
self.native.measure(View.MeasureSpec.UNSPECIFIED, View.MeasureSpec.UNSPECIFIED)
min_height = self.native.getMeasuredHeight()
self.interface.intrinsic.height = min_height
self.interface.intrinsic.height = self.scale_out(min_height, ROUND_UP)
# Ask it how wide it would be if it had to be the minimum height.
self.native.measure(
View.MeasureSpec.UNSPECIFIED,
View.MeasureSpec.makeMeasureSpec(min_height, View.MeasureSpec.AT_MOST),
)
self.interface.intrinsic.width = at_least(self.native.getMeasuredWidth())
self.interface.intrinsic.width = self.scale_out(
at_least(self.native.getMeasuredWidth()), ROUND_UP
)

def set_alignment(self, value):
self.set_textview_alignment(value, Gravity.TOP)
1 change: 1 addition & 0 deletions android/src/toga_android/widgets/multilinetextinput.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ def _on_lose_focus(self):
def set_alignment(self, value):
self.set_textview_alignment(value, Gravity.TOP)

# This method is necessary to override the TextInput base class.
def rehint(self):
self.interface.intrinsic.width = at_least(self.interface._MIN_WIDTH)
self.interface.intrinsic.height = at_least(self.interface._MIN_HEIGHT)
Expand Down
10 changes: 8 additions & 2 deletions android/src/toga_android/widgets/progressbar.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from decimal import ROUND_UP

from android import R
from android.view import View
from android.widget import ProgressBar as A_ProgressBar
Expand Down Expand Up @@ -92,5 +94,9 @@ def rehint(self):
View.MeasureSpec.UNSPECIFIED,
View.MeasureSpec.UNSPECIFIED,
)
self.interface.intrinsic.width = at_least(self.native.getMeasuredWidth())
self.interface.intrinsic.height = self.native.getMeasuredHeight()
self.interface.intrinsic.width = self.scale_out(
at_least(self.native.getMeasuredWidth()), ROUND_UP
Copy link
Member

Choose a reason for hiding this comment

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

Should this have a MIN_WIDTH component to ensure it doesn't just collapse to the smallest possible rendered size?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, that would probably be better, and more consistent with some of the other backends. I didn't do that in this PR because I wanted to limit it to just one problem at a time.

)
self.interface.intrinsic.height = self.scale_out(
self.native.getMeasuredHeight(), ROUND_UP
)
5 changes: 0 additions & 5 deletions android/src/toga_android/widgets/scrollcontainer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
from android.view import Gravity, View
from android.widget import HorizontalScrollView, LinearLayout, ScrollView
from java import dynamic_proxy
from travertino.size import at_least

from ..container import Container
from .base import Widget
Expand Down Expand Up @@ -103,7 +102,3 @@ def set_position(self, horizontal_position, vertical_position):

def set_background_color(self, value):
self.set_background_simple(value)

def rehint(self):
self.interface.intrinsic.width = at_least(0)
self.interface.intrinsic.height = at_least(0)
10 changes: 8 additions & 2 deletions android/src/toga_android/widgets/selection.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from decimal import ROUND_UP

from android import R
from android.view import View
from android.widget import AdapterView, ArrayAdapter, Spinner
Expand Down Expand Up @@ -84,5 +86,9 @@ def clear(self):

def rehint(self):
self.native.measure(View.MeasureSpec.UNSPECIFIED, View.MeasureSpec.UNSPECIFIED)
self.interface.intrinsic.width = at_least(self.native.getMeasuredWidth())
self.interface.intrinsic.height = self.native.getMeasuredHeight()
self.interface.intrinsic.width = self.scale_out(
at_least(self.native.getMeasuredWidth()), ROUND_UP
mhsmith marked this conversation as resolved.
Show resolved Hide resolved
)
self.interface.intrinsic.height = self.scale_out(
self.native.getMeasuredHeight(), ROUND_UP
)
10 changes: 8 additions & 2 deletions android/src/toga_android/widgets/slider.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from decimal import ROUND_UP

from android import R
from android.view import View
from android.widget import SeekBar
Expand Down Expand Up @@ -67,5 +69,9 @@ def _load_tick_drawable(self):

def rehint(self):
self.native.measure(View.MeasureSpec.UNSPECIFIED, View.MeasureSpec.UNSPECIFIED)
self.interface.intrinsic.width = at_least(self.native.getMeasuredWidth())
self.interface.intrinsic.height = self.native.getMeasuredHeight()
self.interface.intrinsic.width = self.scale_out(
at_least(self.native.getMeasuredWidth()), ROUND_UP
mhsmith marked this conversation as resolved.
Show resolved Hide resolved
)
self.interface.intrinsic.height = self.scale_out(
self.native.getMeasuredHeight(), ROUND_UP
)
10 changes: 8 additions & 2 deletions android/src/toga_android/widgets/switch.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from decimal import ROUND_UP

from android.view import View
from android.widget import CompoundButton, Switch as A_Switch
from java import dynamic_proxy
Expand Down Expand Up @@ -43,5 +45,9 @@ def set_value(self, value):

def rehint(self):
self.native.measure(View.MeasureSpec.UNSPECIFIED, View.MeasureSpec.UNSPECIFIED)
self.interface.intrinsic.width = at_least(self.native.getMeasuredWidth())
self.interface.intrinsic.height = self.native.getMeasuredHeight()
self.interface.intrinsic.width = self.scale_out(
at_least(self.native.getMeasuredWidth()), ROUND_UP
)
self.interface.intrinsic.height = self.scale_out(
self.native.getMeasuredHeight(), ROUND_UP
)
5 changes: 0 additions & 5 deletions android/src/toga_android/widgets/table.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
from android.view import Gravity, View
from android.widget import LinearLayout, ScrollView, TableLayout, TableRow, TextView
from java import dynamic_proxy
from travertino.size import at_least

import toga

Expand Down Expand Up @@ -221,7 +220,3 @@ def set_background_color(self, value):
def set_font(self, font):
self._font_impl = font._impl
self.change_source(self.interface.data)

def rehint(self):
self.interface.intrinsic.width = at_least(self.interface._MIN_WIDTH)
self.interface.intrinsic.height = at_least(self.interface._MIN_HEIGHT)
8 changes: 5 additions & 3 deletions android/src/toga_android/widgets/textinput.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
from decimal import ROUND_UP

from android.text import InputType, TextWatcher
from android.view import Gravity, View
from android.widget import EditText
from java import dynamic_proxy
from travertino.size import at_least

from toga_android.keys import toga_key

Expand Down Expand Up @@ -123,6 +124,7 @@ def _on_lose_focus(self):
self.interface.on_lose_focus()

def rehint(self):
self.interface.intrinsic.width = at_least(self.interface._MIN_WIDTH)
self.native.measure(View.MeasureSpec.UNSPECIFIED, View.MeasureSpec.UNSPECIFIED)
Copy link
Member

Choose a reason for hiding this comment

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

Why no width specifier?

Copy link
Member Author

Choose a reason for hiding this comment

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

The base class refresh method (as opposed to rehint) already specifies a default width and height, so we only need to assign whichever one(s) we want to override.

self.interface.intrinsic.height = self.native.getMeasuredHeight()
self.interface.intrinsic.height = self.scale_out(
self.native.getMeasuredHeight(), ROUND_UP
)
5 changes: 0 additions & 5 deletions android/src/toga_android/widgets/webview.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

from android.webkit import ValueCallback, WebView as A_WebView, WebViewClient
from java import dynamic_proxy
from travertino.size import at_least

from toga.widgets.webview import JavaScriptResult

Expand Down Expand Up @@ -81,7 +80,3 @@ def evaluate_javascript(self, javascript, on_result=None):
javascript, ReceiveString(result.future, on_result)
)
return result

def rehint(self):
self.interface.intrinsic.width = at_least(self.interface._MIN_WIDTH)
self.interface.intrinsic.height = at_least(self.interface._MIN_HEIGHT)
1 change: 1 addition & 0 deletions changes/2186.misc.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Correct scaling of MIN_WIDTH and MIN_HEIGHT on Android and WinForms
3 changes: 3 additions & 0 deletions core/src/toga/widgets/box.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@


class Box(Widget):
_MIN_WIDTH = 0
_MIN_HEIGHT = 0

def __init__(
self,
id: str | None = None,
Expand Down
3 changes: 3 additions & 0 deletions core/src/toga/widgets/canvas.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1181,6 +1181,9 @@ def __call__(self, widget: Canvas, width: int, height: int, **kwargs):


class Canvas(Widget):
_MIN_WIDTH = 0
_MIN_HEIGHT = 0

def __init__(
self,
id=None,
Expand Down
3 changes: 3 additions & 0 deletions core/src/toga/widgets/scrollcontainer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@


class ScrollContainer(Widget):
_MIN_WIDTH = 0
_MIN_HEIGHT = 0
Copy link
Member

Choose a reason for hiding this comment

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

Not 100% convinced this is desirable. Box and Canvas both need to be able to collapse to 0 in case someone wants to use them for very small or 0 sized display elements - but ScrollContainer has to have scroll bars. Assuming a minimum size (historically 100x100, but I'm open to other suggestions) would seem like a sensible default, as otherwise you're potentially going to have a scroll container that can collapse to 0 size in a layout, and therefore be unusable.

Any user-specified size will always override this value - so if the use explicitly asks for a 10x10 scroll container, then they will get it - but a minimum value puts a floor on how small Pack will let it get by default.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed.


def __init__(
self,
id=None,
Expand Down
15 changes: 5 additions & 10 deletions winforms/src/toga_winforms/widgets/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from abc import ABC, abstractmethod
from decimal import ROUND_HALF_EVEN, ROUND_UP, Decimal
from decimal import ROUND_HALF_EVEN, Decimal

from System.Drawing import (
Color,
Expand Down Expand Up @@ -144,15 +144,10 @@ def remove_child(self, child):
child.container = None

def refresh(self):
intrinsic = self.interface.intrinsic
intrinsic.width = intrinsic.height = None
# Default values; may be overwritten by rehint().
self.interface.intrinsic.width = at_least(self.interface._MIN_WIDTH)
self.interface.intrinsic.height = at_least(self.interface._MIN_HEIGHT)
mhsmith marked this conversation as resolved.
Show resolved Hide resolved
self.rehint()
assert intrinsic.width is not None
assert intrinsic.height is not None

intrinsic.width = self.scale_out(intrinsic.width, ROUND_UP)
intrinsic.height = self.scale_out(intrinsic.height, ROUND_UP)

@abstractmethod
def rehint(self):
...
pass
5 changes: 0 additions & 5 deletions winforms/src/toga_winforms/widgets/box.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,8 @@
import System.Windows.Forms as WinForms
from travertino.size import at_least

from .base import Widget


class Box(Widget):
def create(self):
self.native = WinForms.Panel()

def rehint(self):
self.interface.intrinsic.width = at_least(0)
self.interface.intrinsic.height = at_least(0)
12 changes: 8 additions & 4 deletions winforms/src/toga_winforms/widgets/button.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from decimal import ROUND_UP

import System.Windows.Forms as WinForms
from travertino.size import at_least

Expand Down Expand Up @@ -31,7 +33,9 @@ def set_text(self, text):
self.native.Text = text

def rehint(self):
# self.native.Size = Size(0, 0)
# print("REHINT Button", self, self.native.PreferredSize)
self.interface.intrinsic.width = at_least(self.native.PreferredSize.Width)
self.interface.intrinsic.height = self.native.PreferredSize.Height
self.interface.intrinsic.width = self.scale_out(
at_least(self.native.PreferredSize.Width), ROUND_UP
Copy link
Member

Choose a reason for hiding this comment

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

Should this have a MIN_SIZE component?

Copy link
Member Author

Choose a reason for hiding this comment

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

In this case I think it's better to use PreferredSize, because it reacts to the label text.

)
self.interface.intrinsic.height = self.scale_out(
self.native.PreferredSize.Height, ROUND_UP
)
Loading