Skip to content

Conversation

@johnzhou721
Copy link
Contributor

@johnzhou721 johnzhou721 commented Jan 3, 2026

Adds toolbar support to iOS MainWindows.

All caveats and user cautions documented inline.

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 January 3, 2026 23:06
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.

Whenever I'm reviewing a PR, my first question is "How do I test this?". I can confirm that the Command example does work; but with a few weird glitches - which possibly fall inside the boundaries of the platform notes you've documented

But that's pointing at the bigger issue here - you've implemented a bunch of logic, making a bunch of design decisions, without actually confirming if those design decisions are appropriate or acceptable. The fact that you've got a multiple paragraphs of platform notes, and no other platform requires those notes should be an indicator of something.

This is a non-trivial body of work, being where there isn't a straightforward "implement the same API on a new platform". The first step is to propose a course of action, not implement it. An implementation could be a useful proof of concept - but only if that's the easiest path to illustrate the proposal.

So - in this case, I suggest stepping back a bit and at least documenting the concerns here so we can have a conversation about the design decisions you've made.

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