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

[widget Audit] toga.DetailedList #2025

Merged
merged 33 commits into from
Sep 9, 2023

Conversation

freakboy3742
Copy link
Member

@freakboy3742 freakboy3742 commented Jul 4, 2023

Audit of DetailedList.

Builds on #2017 because of dependencies on Table and Sources (especially on GTK).

  • Adds the ability to customise the accessors for data.
  • Adds the ability to customise the missing value for data.
  • Changes the default interpretation order when populating with a list of tuples, or a list of lists. This is to ensure that the first value in each item is the "title", which means a list of strings will produce a usable DetailedList. This shouldn't impact too many people as the only documented way of feeding a DetailedList was a list of dicts, which is unaffected by this change.
  • On iOS, the names "Delete" and "Remove" are interpreted as "destructive" actions, and rendered with a red background in accordance with iOS style guides. There's potential to do a similar thing on GTK, showing icons rather than text for certain "magic" words, but I've left that as future work.
  • I've added support for a --coverage flag when running the testbed; I was finding it cumbersome to have to run the entire test suite just to get coverage of a single widget.

Fixes:

Audit checklist

  • Core tests ported to pytest
  • Core tests at 100% coverage
  • Core docstrings complete, and in Sphinx format
  • Documentation complete and consistently formatted
  • cocoa backend at 100% coverage
  • winforms backend at 100% coverage
  • gtk backend at 100% coverage
  • iOS backend at 100% coverage
  • android backend at 100% coverage
  • Widget support table updated
  • Widget Issue backlog reviewed

@freakboy3742 freakboy3742 changed the title Audit detailedlist [widget Audit] toga.DetailedList Jul 4, 2023
@freakboy3742 freakboy3742 requested a review from mhsmith August 4, 2023 06:17
@freakboy3742 freakboy3742 marked this pull request as ready for review August 29, 2023 22:56
Copy link
Member Author

@freakboy3742 freakboy3742 left a comment

Choose a reason for hiding this comment

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

A couple of minor suggestions, but otherwise looking good (other than the single failing test)

android/src/toga_android/widgets/base.py Outdated Show resolved Hide resolved
android/src/toga_android/widgets/detailedlist.py Outdated Show resolved Hide resolved
winforms/src/toga_winforms/widgets/detailedlist.py Outdated Show resolved Hide resolved
docs/reference/api/widgets/detailedlist.rst Outdated Show resolved Hide resolved
Comment on lines 236 to 241
async def add_row(event_widget, **kwargs):
def add_row(event_widget, **kwargs):
assert event_widget == widget
assert kwargs == {}

# Simulate a reload delay
await probe.redraw("Wait for simulated reload")
Copy link
Member

Choose a reason for hiding this comment

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

This made the test unreliable in --slow mode, because the handler and the test function are running in separate asyncio tasks and there's no guarantee of which one will finish awaiting first.

@freakboy3742 freakboy3742 merged commit 813e682 into beeware:main Sep 9, 2023
40 checks passed
@freakboy3742 freakboy3742 deleted the audit-detailedlist branch September 9, 2023 23:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment