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

Added support for current_window setter. #1887

Merged
merged 7 commits into from
Apr 24, 2023
Merged

Conversation

proneon267
Copy link
Contributor

An API for setting a window into active focus was added.

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

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.

The general shape of this makes sense; I'm pretty sure the cocoa implementation shouldn't be much more than window.native.makeKeyAndFront().

However, the piece that is definitely missing is a test of this behavior. Ideally, this would be an automated test; but at the very least, we need a mechanism to exercise this functionality to demonstrate that it works.

The easiest way I can think of to test this would be an update to the examples/window app. There's a test action that creates multiple sub-windows; adding an extra action that will cycle the current window between the sub-windows a couple of times, and will also report them current window to the label in the main window.

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.

Thanks - I've made a couple more tweaks (one to correct the specific API invocation on Cocoa, and one to make the GUI test more visible); but otherwise, this looks good to go.

@proneon267
Copy link
Contributor Author

Thanks

@freakboy3742 freakboy3742 merged commit b6a85fa into beeware:main Apr 24, 2023
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.

2 participants