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

macOS use AutoLayout in utility bars #3326

Merged

Conversation

sweetppro
Copy link
Collaborator

Fixes #3198

@ckerr ckerr requested a review from nevack June 20, 2022 04:16
@sweetppro sweetppro changed the title macOS use constraints in utility bars macOS use AutoLayou in utility bars Jun 20, 2022
@sweetppro sweetppro changed the title macOS use AutoLayou in utility bars macOS use AutoLayout in utility bars Jun 20, 2022
@nevack
Copy link
Member

nevack commented Jun 20, 2022

Honestly, I have mixed feelings about that.

It's really great we can delegate all layout related calculations to AppKit.

What really bothers me it that managing/reviewing XIBs is really hard.

P.S. I hope we can adopt something like SwiftUI in the future.
I really like XML layouting in Android. It's easy and structured well, but XIBs is PITA

@DevilDimon
Copy link
Collaborator

We can agree to only do layout in code for new views and rewrite old xibs to use code on significant refactors like this. Might be an issue on bigger views though

@sweetppro
Copy link
Collaborator Author

Does no one actually compile and run apps when reviewing?
A fresh build takes less than a min for me

@ckerr
Copy link
Member

ckerr commented Jun 21, 2022

Does no one actually compile and run apps when reviewing? A fresh build takes less than a min for me

Could you rephrase this? I'd like to understand your point, but am missing it: building doesn't seem to be necessary for either of the review comments, which are discussion on what layout strategy the project should use..?

@sweetppro
Copy link
Collaborator Author

Does no one actually compile and run apps when reviewing? A fresh build takes less than a min for me

Could you rephrase this? I'd like to understand your point, but am missing it: building doesn't seem to be necessary for either of the review comments, which are discussion on what layout strategy the project should use..?

Ah it was mentioned that it’s hard to review code that has been generated by Xcode. But this PR is a UI change, and I think it should be reviewed manually by compiling and testing rather than eyeballing the code changes.

@sweetppro
Copy link
Collaborator Author

The reason I submitted the PR was to have less code, not just a refactor. Autolayout is a wonderful code free tool when done vie interface builder(Xcode), it would be a shame not to use it.

@ckerr
Copy link
Member

ckerr commented Jun 21, 2022

Gotcha. Thanks for the clarification.

@ckerr
Copy link
Member

ckerr commented Jun 22, 2022

@nevack @DevilDimon it sounds like you two would generally avoid more xib use in the mac client due to its complexity, is that correct?

I'm tryng to understand what the resolution should be for this PR.

@nevack
Copy link
Member

nevack commented Jun 22, 2022

@nevack @DevilDimon it sounds like you two would generally avoid more xib use in the mac client due to its complexity, is that correct?

I'm tryng to understand what the resolution should be for this PR.

IMHO, XIBs are really hard to manage.

  1. They are difficult to review.
  2. It's impossible to merge in case of conflicts.
  3. They are touched by Xcode every time I open them in some different env, like on external display.

I don't have experience in code-only UI.
I want to listen to some other opinions.

Regarding this PR - LGTM.
We cannot do anything about using XIBs right now.

@ckerr ckerr force-pushed the macOS-use-constraints-in-utility-bars branch from 8642a62 to 83801c3 Compare June 22, 2022 18:04
@ckerr ckerr merged commit 286d438 into transmission:main Jun 23, 2022
@sweetppro sweetppro deleted the macOS-use-constraints-in-utility-bars branch June 23, 2022 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

macOS - Filter bar: align along the symmetry axis
4 participants