Skip to content

Learn and remember verified window size limits - #1902

Closed
kiteretsu903 wants to merge 1 commit into
rxhanson:mainfrom
kiteretsu903:fix/review-size-limits
Closed

kiteretsu903 wants to merge 1 commit into
rxhanson:mainfrom
kiteretsu903:fix/review-size-limits

Conversation

@kiteretsu903

@kiteretsu903 kiteretsu903 commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Some applications accept a resize request but settle at a larger size. This change learns from that actual result so subsequent placement can account for the window's size limits. “Remember window size limits” is enabled by default and provides a list for inspecting and clearing learned values.

Behavior

  • A verified, settled clamp is sufficient to learn a limit; it does not require two identical attempts. Pending, cancelled, or unrelated resize results must not become evidence.
  • Learned values are hints, not permanent restrictions. A later successful resize to a smaller size can lower the remembered limit, and a newly verified larger minimum can raise it. Resize requests can still test whether an older hint is valid.
  • Live window/process identity and persisted application identity are handled separately so recycled identifiers or outdated records are not blindly applied to another window.
  • Disabling remembering clears learned state; explicitly reported Accessibility constraints remain distinct from learned evidence. Users can also clear entries through the size-limit list.

Implementation and dependency

Observation, evidence evaluation, persistence, and the settings list are grouped under the size-limit feature. Placement supplies the requested and settled geometry; it does not own the archive or wait for the settings UI. Warnings are presented separately from resize execution.

This prepared change uses the isolated animation and ordinary-placement results to observe completion consistently. Remembering does not require animation to be enabled. Fit remaining space and divider behavior are separate changes.

Validation

The full branch XCTest suite passed (538 tests, including existing tests). Combined native divider checks confirmed first-clamp learning, repeat use, and increasing limits.

The remaining native checks for decreasing limits, persistence across restart, and new window identities are pending; their automated coverage should not be read as completed application-level E2E coverage.

This replaces the corresponding part of #1887.

@kiteretsu903
kiteretsu903 force-pushed the fix/review-size-limits branch from 726a819 to bd8538e Compare October 10, 2026 02:51
@kiteretsu903
kiteretsu903 marked this pull request as ready for review October 10, 2026 02:51
@rxhanson

rxhanson commented Oct 12, 2026 •

Copy link
Copy Markdown
Owner

This change learns from that actual result so subsequent placement can account for the window's size limits.

Rephrasing this in my words: The value in knowing that a window cannot be resized smaller prior to placing it, is that on subsequent attempts to place a window we could treat it as "fixed width or height" without re-applying a placement and showing window jitter.

I think a common scenario that we could end up with is someone wondering why their window size is being limited by Rectangle, and not bothering or wanting to be bothered with managing Rectangle's learned limits in the UI. I'm inclined to err on the side of keeping Rectangle a little simpler both in code and in the UI, and not rolling this one in.

Also there are a number of edge cases to maintain with UI changes like this - for example, if I "Reset Application", then hit a window limit, I noticed that the Application/Window does not get re-added.

On this topic, the reason why the Minimum window size reached notification is not enabled by default is because it will show erroneously for Terminal windows when the sizing of the window is determined by character sizes. Once that's fixed I'd like to enable that setting by default. (I know that you didn't write that feature, but figured I'd explain.)

Edit: Ok, looking at later pull requests I can see where it's desired to tell if a window will fit into a space prior to placing it. There is benefit to showing a footprint for a window knowing precisely how small the window can be, and how much it can fill. I'm still not sold on having this in the UI, though. Maybe there's still a better way that this problem can be solved?

@kiteretsu903

Copy link
Copy Markdown
Contributor Author

@rxhanson Hey Ryan, thanks for your feedback as always. In that case, I think it’s reasonable to drop this PR. I can adjust the later PRs so they no longer depend on it. let me know if that works for you!

@rxhanson

Copy link
Copy Markdown
Owner

I was thinking about this a little more, and from the standpoint of the footprint window it might be fine to have a stale read on the minimum size of a window, as the placement would still end up with the minimum fit anyhow. I think if we were to walk that path, it would look like just another toggle for tracking minimum window sizes, as opposed to the table, and the minimum size could be used simply for the footprint. I'll close this one out since a change like that would be best done as a new change.

@rxhanson rxhanson closed this Oct 14, 2026
@kiteretsu903

Copy link
Copy Markdown
Contributor Author

I was thinking about this a little more, and from the standpoint of the footprint window it might be fine to have a stale read on the minimum size of a window, as the placement would still end up with the minimum fit anyhow. I think if we were to walk that path, it would look like just another toggle for tracking minimum window sizes, as opposed to the table, and the minimum size could be used simply for the footprint. I'll close this one out since a change like that would be best done as a new change.

This makes sense to me!

@kiteretsu903

Copy link
Copy Markdown
Contributor Author

@rxhanson I've removed the dependency on the limited window PR for all remaining PRs

@kiteretsu903
kiteretsu903 deleted the fix/review-size-limits branch October 14, 2026 04:38
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