Skip to content

Try to fix GTK4 DateInput tesst#4040

Merged
HalfWhitt merged 14 commits into
beeware:mainfrom
johnzhou721:patch-31
Jan 3, 2026
Merged

Try to fix GTK4 DateInput tesst#4040
HalfWhitt merged 14 commits into
beeware:mainfrom
johnzhou721:patch-31

Conversation

@johnzhou721

@johnzhou721 johnzhou721 commented Dec 31, 2025

Copy link
Copy Markdown
Contributor

First confirming non-intermittent by making a blank PR here.

After this PR fails CI I will start to investigate.

PR Checklist:

  • All new features have been tested
  • All new features have been documented
  • I have read the CONTRIBUTING.md file
  • I will abide by the code of conduct

@johnzhou721 johnzhou721 marked this pull request as ready for review December 31, 2025 05:19
@johnzhou721 johnzhou721 marked this pull request as draft December 31, 2025 18:58
@johnzhou721 johnzhou721 marked this pull request as ready for review December 31, 2025 19:36
@johnzhou721

Copy link
Copy Markdown
Contributor Author

OK... explaining this PR now that it's ready for review (finally):

The GTK4 version in CI does not seem to emit a signal when programatically changing the date but only changing the year. The last testcase is 9999/12/31, and guess what today is in UTC... 2025/12/31! So the signal does not emit.

This PR does 2 things:

  1. Manually call gtk_on_change instead of relying on the signal when programatically changing. this is the correct fix since the documentation says "when the user selects a day" (https://docs.gtk.org/gtk4/signal.Calendar.day-selected.html), so manually calling the handler while suppressing undocumented signal emission from programmatic change is the correct approach.
  2. Add a only-year-changing testcase to the data. That way we can consistently test this behavior on days that aren't 12/31/2025.

It might be time to celebrate here, as it's the end of the year!!! We've accomplished so much in 2025, like GTK4 support for basic widgets (which revealed the issue), a Qt backend, web testing prototypes, plus massive things in Briefcase.

But this also implies that the core team is on holiday -- and I'm aware that I may not get a response until up to 8 days.

@HalfWhitt

Copy link
Copy Markdown
Member

Does this change mean that if you set the date to the same date that it currently is, a change signal will be emitted? It doesn't look like we're currently testing that case.

@johnzhou721

Copy link
Copy Markdown
Contributor Author

@HalfWhitt Good catch, I will fix it when I get home today.

@johnzhou721

Copy link
Copy Markdown
Contributor Author

@HalfWhitt It seems that there is a much more widespread problem regarding this in Toga, see TextInput, DateInput, and TimeInputs on iOS and Android, those all fire signals manually without chceking if the contents actually changed. Same with canvas on some backends, the resizing fires on set_bounds, but the bounds may not necessarily have changed (since it might just be a spurious relayout.)

Do you think we should handle this separately and not at part of this PR after we make a decision on whether this is severe enough? Since this affects other widgets and is not specific to GTK.

@mhsmith

mhsmith commented Jan 2, 2026

Copy link
Copy Markdown
Member

We've already documented that when a "property is set to its existing value, [...] whether it generates an event is undefined". Some backends do, and some don't, so I don't think it would be worth the effort of regularizing this.

@HalfWhitt

Copy link
Copy Markdown
Member

We've already documented that when a "property is set to its existing value, [...] whether it generates an event is undefined". Some backends do, and some don't, so I don't think it would be worth the effort of regularizing this.

Fair enough, I'd missed that part... It might, at some point, be worth picking either yea or nay, and ensuring consistency of that as far as what a Toga interface user's code sees, regardless or what is or isn't happening at the OS level. That said, it's definitely a separate issue from this one.

@johnzhou721

Copy link
Copy Markdown
Contributor Author

@mhsmith @HalfWhitt Ack; sorry for not seeing the discussion here for 2+ hours.

In that case... any more concerns?

@HalfWhitt HalfWhitt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ack; sorry for not seeing the discussion here for 2+ hours.

Asynchronous communication is perfectly normal and expected. : )

The logic here looks fine, and good catch on the root cause of the failures. Just one typo caught, and a question on how to "spell" this...

Comment thread gtk/src/toga_gtk/widgets/dateinput.py Outdated
Comment thread gtk/src/toga_gtk/widgets/dateinput.py Outdated
johnzhou721 and others added 3 commits January 2, 2026 17:05
Co-authored-by: Charles Whittington <CharlesWhitt@gmail.com>
@johnzhou721 johnzhou721 requested a review from HalfWhitt January 2, 2026 23:36
Comment thread gtk/src/toga_gtk/widgets/dateinput.py
@johnzhou721 johnzhou721 requested a review from HalfWhitt January 3, 2026 00:09

@HalfWhitt HalfWhitt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good to me — thank you for the clarification re: GTK's signals vs. Toga's events.

@HalfWhitt HalfWhitt merged commit 6f00267 into beeware:main Jan 3, 2026
56 of 57 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants