macOS fix window drawing behaviour

[sweetppro]

• updated MainMenu.xib to use autolayout
• fixed a number of regressions from macOS - fix some window drawing issues #3278 re-add macOS Fullscreen support #3261
  * removed unused GearshapeTemplate from macoxs/CMakeLists.txt

Fixes #3287

Fixes #3282

Fixes #3281

Fixes #3234

Fixes #3215

[GaryElshaw]

Nice work e hoa.

If you have some time i'd like to know what you think. #3295

[sweetppro]

note, that this PR does actually keep Full Screen support if thats not entirely clear.
I just decided to go back to the drawing board, and start fresh rather than try to make the current code base work...

Ive implemented some auto layout into the main window, and utilised constraints to get rid of a a bunch of UI calculations.

[nevack]

Please, do not batch your changes like this.

1. Use separate commits (even separate PRs, if eligible).
2. Use actual git revert.

[GaryElshaw]

Please, do not batch your changes like this.

1. Use separate commits (even separate PRs, if eligible).
2. Use actual git revert.

@nevack That's not a review, it's unneeded criticism. What do you think of the substance of the code changes?

[nevack]

That's not a review, it's unneeded criticism. What do you think of the substance of the code changes?

I do not agree that it's unneeded criticism. It's just a suggestion to make review easier.

Also, I do think it's a must to have proper git revert commits, to have them separate in history.

I have taken a look at some parts of diff.

I think it is a dead end to keep even more code in Controller.
What do you think about moving some logic to new NSWindowCotroller subclass and have all UI-related work separate in it?

[GaryElshaw]

Controller is, i think, a behemoth. But i'm also biased in that i think there have been so many piecemeal changes that a proper code-review is needed and some good discussion. I think everyone is on the same page that the macOS client needs modernisation and is dedicated to doing it, but how that is implemented with the current codebase might have equally different answers.

Everyone has their own feeling about prioritisation. For example, this is mine for 4.0. I want the gui to be consistent across macOS #3295

Oh, and this is a blocker, i think: (#2705)

Edit: As an aside, if the only option to change code for this particular PR is based in Controller, then that's the only option. I doubt anyone is going to disagree that Controller should be disaggregated.

[sweetppro]

How about we just get T in a functional state before we discuss more refactoring.

[GaryElshaw]

No disagreement. But i think you may have missed what i was saying. Everyone is working with what exists and often that's a cludge. @ckerr is often at-ing me about png. That's cool, but the codebase won't allow svg, so we have what we have.

[livings124]

I'm going to agree with @nevack and also request that changes be more reviewable in smaller PRs. If a refactor is done, Controller really does need to be broken up. And I would also advice against more large changes before the next pending major release.

[nevack]

For example, this is mine for 4.0

I already said, that we need to revert recent changes related to Toolbar/Full screen and iterate again over these bugs.
Major refactorings like this can be hard to test.
We are already in the state when fixes cause more serious bugs, than we had, IMHO

[ckerr]

OK based on the feedback here I'm going to mark this as 'Request changes'....... @sweetppro would it be possible to stack this into smaller PRs for reviews?

If you're new to doing this in GitHub it's actually very easy: fix bug A in branch A and PR A to be merged into main. Then fix bug B in branch B which is branched off of A, and PR B to be merged into A. Repeat as necessary.

The nice thing is that when PR A gets merged into main, PR B will automatically get set to target main. This makes a nice queue of A, B, ... all pointing at main, even when B depends on A and C depends on B, etc.

[sweetppro]

OK based on the feedback here I'm going to mark this as 'Request changes'....... @sweetppro would it be possible to stack this into smaller PRs for reviews?

ok sure,
Ill close this and submit 3 seperate PRs - ive already moved the CMakeFileList change into its own seperate PR #3301

• 1 to revert the toolbar change
• 1 to revert fullscreen changes
• and a final one to submit my new changes
  I am definitely not a GitHub wizard as I don't use it except for working on open source projects, so will do my best

[ckerr]

Controller is, i think, a behemoth. But i'm also biased in that i think there have been so many piecemeal changes that a proper code-review is needed and some good discussion. I think everyone is on the same page that the macOS client needs modernisation and is dedicated to doing it, but how that is implemented with the current codebase might have equally different answers.

Everyone has their own feeling about prioritisation. For example, this is mine for 4.0. I want the gui to be consistent across macOS #3295

My first thought on these comments was negative, e.g. I really don't think macOS 10 is the priority for 4.0 and so I'm more negative about 3295 right now than I would've been, say, a couple of months ago.

But I think there is a good point here, and that is the question of what is the best / shortest path to get the macOS GUI stable and ready for public release? As I've said many times I'm not the macOS coder on the project so I don't really have the context to answer this. My goal is to have 4.0.0-beta.1 ready in < 1 month, so we shouldn't take on anything that can't get completed in that timeline.

[sweetppro]

@ckerr if we drop macOS10 support, #3295 would be unnecessary

[GaryElshaw]

My goal is to have 4.0.0-beta.1 ready in < 1 month

Thanks Charles, that's the window.