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

add a layout debug mode that sets background colors #2953

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions changes/2953.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Toga now has a layout debugging mode. If you set ``TOGA_DEBUG_LAYOUT=1`` in your app's runtime environment, widgets will be rendered with different background colors, making it easier to identify how space is being allocated by Toga's layout algorithm.
33 changes: 33 additions & 0 deletions core/src/toga/widgets/base.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
from __future__ import annotations

from builtins import id as identifier
from os import environ
from random import shuffle
from typing import TYPE_CHECKING, Any, TypeVar
from warnings import warn

Expand All @@ -17,10 +19,29 @@
StyleT = TypeVar("StyleT", bound=BaseStyle)


# based on colors from https://davidmathlogic.com/colorblind
debug_background_palette = [
Copy link
Member

Choose a reason for hiding this comment

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

If this is a list of constants, it should be treated as a constant by naming convention:

Suggested change
debug_background_palette = [
DEBUG_BACKGROUND_PALETTE = [

"#d0e2ed", # very light blue
"#b8d2e9", # light blue
"#f8ccb0", # light orange
"#f6d3be", # soft orange
"#c7e7b2", # light green
"#f0b2d6", # light pink
"#e5dab0", # light yellow
"#d5c2ea", # light lavender
"#b2e4e5", # light teal
"#e5e4af", # light cream
"#bde2dc", # soft turquoise
]
shuffle(debug_background_palette)
Copy link
Member

Choose a reason for hiding this comment

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

Why programmatically shuffle? We definitely need an order which cycles good contrast (i.e., we don't have "light blue" next to "very light blue"), but that order doesn't need to be random.

Plus, randomisation takes more time at startup, and a randomised order could be jarring for as successive runs won't give the same output.



class Widget(Node):
_MIN_WIDTH = 100
_MIN_HEIGHT = 100

_debug_color_index = 0

def __init__(
self,
id: str | None = None,
Expand All @@ -34,6 +55,18 @@ def __init__(
:param style: A style object. If no style is provided, a default style
will be applied to the widget.
"""
# If the object has _USE_DEBUG_BACKGROUND=True and layout debug mode
# is on, change bg color.BufferError
if getattr(self, "_USE_DEBUG_BACKGROUND", False):
if environ.get("TOGA_DEBUG_LAYOUT") == "1":
Widget._debug_color_index += 1
style = style if style else Pack()
style.background_color = debug_background_palette[
Widget._debug_color_index % len(debug_background_palette)
]
else:
self._USE_DEBUG_BACKGROUND = False

super().__init__(style=style if style is not None else Pack())

self._id = str(id if id else identifier(self))
Expand Down
2 changes: 2 additions & 0 deletions core/src/toga/widgets/box.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ def __init__(
will be applied to the widget.
:param children: An optional list of children for to add to the Box.
"""
# enable the debug background functionality
self._USE_DEBUG_BACKGROUND = True
super().__init__(id=id, style=style)

# Children need to be added *after* the impl has been created.
Expand Down
2 changes: 2 additions & 0 deletions core/src/toga/widgets/optioncontainer.py
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,8 @@ def __init__(
<OptionContainerContentT>` to display in the OptionContainer.
:param on_select: Initial :any:`on_select` handler.
"""
# enable the debug background functionality
self._USE_DEBUG_BACKGROUND = True
super().__init__(id=id, style=style)
self._content = OptionList(self)
self.on_select = None
Expand Down
2 changes: 2 additions & 0 deletions core/src/toga/widgets/scrollcontainer.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ def __init__(
:param on_scroll: Initial :any:`on_scroll` handler.
:param content: The content to display in the scroll window.
"""
# enable the debug background functionality
self._USE_DEBUG_BACKGROUND = True
super().__init__(id=id, style=style)

self._content: Widget | None = None
Expand Down
2 changes: 2 additions & 0 deletions core/src/toga/widgets/splitcontainer.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ def __init__(
:param content: Initial :any:`SplitContainer content <SplitContainerContentT>`
of the container. Defaults to both panels being empty.
"""
# enable the debug background functionality
self._USE_DEBUG_BACKGROUND = True
super().__init__(id=id, style=style)
self._content: list[SplitContainerContentT] = [None, None]

Expand Down
76 changes: 76 additions & 0 deletions core/tests/widgets/test_container_debug_mode.py
Copy link
Member

Choose a reason for hiding this comment

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

We try to keep the test structure matching the widget layout; or, if things are getting complex, matching the feature by the name used in the code. In this case, if there's too many tests to add to widgets/test_base.py, this should probably be widgets/test_debug_layout.py (I think the latter makes sense here)

Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
from pytest import MonkeyPatch
from travertino.colors import color

import toga


# test that a container-like widget in normal mode has a default background
def test_box_no_background_in_normal_mode():
"""A Box has no default background."""
# Disable layout debug mode
with MonkeyPatch.context() as mp:
mp.setenv("TOGA_DEBUG_LAYOUT", "0")
Comment on lines +8 to +12
Copy link
Member

Choose a reason for hiding this comment

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

Monkeypatch can be used as a fixture:

Suggested change
def test_box_no_background_in_normal_mode():
"""A Box has no default background."""
# Disable layout debug mode
with MonkeyPatch.context() as mp:
mp.setenv("TOGA_DEBUG_LAYOUT", "0")
def test_box_no_background_in_normal_mode(monkeypatch):
"""A Box has no default background."""
# Disable layout debug mode
monkeypatch.setenv("TOGA_DEBUG_LAYOUT", "0")

box = toga.Box()
# assert that the bg is default
assert hasattr(box.style, "background_color")
Copy link
Member

Choose a reason for hiding this comment

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

This assertion essentially can't fail. Toga's style framework will ensure that background_color exists as an attribute for read.

assert not box.style.background_color
Copy link
Member

Choose a reason for hiding this comment

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

A check of an actual value, rather than "falseness" would be preferable here - that avoids any odd situations where something like rgb(0,0,0) evaluates as "false" (I don't think it does, but just in case)



# test that a non-container-like widget in layout debug mode has a default background
def test_button_debug_background():
Copy link
Member

Choose a reason for hiding this comment

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

Good idea testing a widget other than Box; - but also worth adding at test for (a) a container widget, and (b) a widget where the debug background won't be applied.

"""A Button in layout debug mode has a default background."""
# Enable layout debug mode
with MonkeyPatch.context() as mp:
mp.setenv("TOGA_DEBUG_LAYOUT", "1")
button = toga.Button()
# assert that the bg is default
assert hasattr(button.style, "background_color")
assert not button.style.background_color
Comment on lines +27 to +28
Copy link
Member

Choose a reason for hiding this comment

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

As above:

  • monkeypatch can be used as a fixture.
  • the hasattr won't be asserting anything
  • A positive value check would be preferable.

The same pattern applies to the subseqeunt tests as well.



# test that a label in layout debug mode has a default background
Copy link
Member

Choose a reason for hiding this comment

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

Why test label and button? Or, if it's important to check every widget (arguable, it is), is there any reason we shouldn't encode this as a test in properties.py like test_enabled/test_enable_noop - that is, a test that can be applied to every widget, with a "uses debug" and "doesn't use debug" variant?

def test_label_no_debug_background():
"""A Label in layout debug mode has a default background."""
# Enable layout debug mode
with MonkeyPatch.context() as mp:
mp.setenv("TOGA_DEBUG_LAYOUT", "1")
label = toga.Label("label")
# assert that the bg is default
assert hasattr(label.style, "background_color")
assert not label.style.background_color


# test that a container-like widget in layout debug mode has a non-default background
# that matches the expected debug_background_palette
def test_box_debug_backgrounds():
"""A Box in layout debug mode has a non-default background."""
# Enable layout debug mode
with MonkeyPatch.context() as mp:
mp.setenv("TOGA_DEBUG_LAYOUT", "1")

boxes = []
debug_bg_palette_length = len(toga.widgets.base.debug_background_palette)
# need enough for coverage of debug_background_palette array index rollover
for i in range(debug_bg_palette_length + 3):
boxes.append(toga.Box())

for counter, box in enumerate(boxes, start=1):
index = counter % debug_bg_palette_length
print(counter)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
print(counter)

assert hasattr(box.style, "background_color")
assert box.style.background_color == color(
toga.widgets.base.debug_background_palette[index]
Copy link
Member

Choose a reason for hiding this comment

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

Unless I'm missing something, this test doesn't account for the state of _debug_color_index at the start of the test. it essentially must be the first (and only?) "background modifying" test in the suite; if any other test enables debug, this test will fail because the colors won't align.

This means either the test needs to monkeypatch _debug_color_index, or the test needs to take the initial state of _debug_color_index into consideration.

)


# test that a scroll container widget in layout debug mode doesn't have
# a default background
def test_scroll_container_debug_background():
"""A container widget has a default background."""
# Disable layout debug mode
with MonkeyPatch.context() as mp:
mp.setenv("TOGA_DEBUG_LAYOUT", "1")
sc = toga.ScrollContainer()
# assert that the bg is not default
assert hasattr(sc.style, "background_color")
assert sc.style.background_color != color("white")
Loading