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

NSView (autoresize): simpler implementation #298

Closed
wants to merge 1 commit into from

Conversation

trunkmaster
Copy link
Member

Change to a simpler and clearer implementation. As a side effect this change fixes Font Panel resize handling.

… side effect this change fixes Font Panel resize handling.
@trunkmaster trunkmaster marked this pull request as draft October 2, 2024 09:02
@trunkmaster trunkmaster marked this pull request as ready for review October 2, 2024 09:17
{
*contentPositionInOut += changePerOption;
}
*contentSizeInOut += (change / 2);
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we divide by 3 if min and max are flexible?

@fredkiefer
Copy link
Member

fredkiefer commented Oct 2, 2024

There is a special test file for autoresizing: NSView_autoresize_and_rounding.m
You could add a test case there that fails before your test and passes afterwards. This would help to better understand the case you are trying to fix.
You will have to run that test separately as currently most tests in gui cannot be run without back.

@trunkmaster
Copy link
Member Author

Thanks for the hint. It seemes that I've made fix to the wrong place. All autoresizing tests fail.

My change introduces different algorithm of autoresizing - it resizes view not proportionally but keeps views aligned to each other without AutoLayout magic. That's why it fixes Font Panel resizing. However it doesn't conform to Cocoa behaviour (checked on macOS and OPENSTEP).

I'll close this PR and make new with Font Panel fix.

@trunkmaster trunkmaster closed this Oct 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants