Skip to content

Conversation

johnzhou721
Copy link
Contributor

Closes #2838 which was inactive for a few months -- tell me if this is too short of a time to assume non-working-on-ness

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 June 20, 2025 02:24
@johnzhou721
Copy link
Contributor Author

Ready for review. Let me know if you want things to keep track of visibility on macOS by subclassing the native class to ensure that start() and stop() are called in the code... but then you'd risk not calling super().start() and super().stop() in your subclass... and the test will still pass... so since there's no native API for visibliity on macOS, there's pretty much no way to test that part.

Copy link
Member

@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 cleanups to code comments, but otherwise this looks good.

I'm not overly concerned about the macOS tests. This test is more about ensuring platforms that require an explicit hide are implementing it correctly; on macOS and GTK, the widget 'does the right thing' out of the box, so additional steps would be overkill.

@freakboy3742 freakboy3742 merged commit aeb6d1b into beeware:main Jun 20, 2025
52 checks passed
@johnzhou721
Copy link
Contributor Author

This test is more about ensuring platforms that require an explicit hide are implementing it correctly;

Thanks for the review. Yes, this is mainly following up on the Winforms PR since I made logic issues there.

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