10.8 Fixes for autolayout

[josh64x2]

The issue is for discussing the autolayout fixes required for 10.8

FYI @Eitot @barijaona

[josh64x2]

@Eitot I've fixed some of the 10.8 stuff by removing all of the manual splitview layout stuff from ArticleListView and using the NSSplitView autosave functionality. There are some constraint issues though and I'm not sure where to start.

The following displays in the console when launching the app:

26/01/17 12:26:35.200 PM    Vienna[270] Unable to simultaneously satisfy constraints:
(
    "<NSAutoresizingMaskLayoutConstraint:0x7f931b02b560 h=-&- v=-&- V:|-(0)-[MessageListView:0x7f931a489940]   (Names: '|':NSClipView:0x7f931a489320 )>",
    "<NSAutoresizingMaskLayoutConstraint:0x7f931b02b5c0 h=-&- v=-&- V:[MessageListView:0x7f931a489940]-(0)-|   (Names: '|':NSClipView:0x7f931a489320 )>",
    "<NSAutoresizingMaskLayoutConstraint:0x7f931a4ab820 h=-&- v=-&- V:|-(17)-[NSClipView:0x7f931a489320]   (Names: '|':NSScrollView:0x7f931a488f10 )>",
    "<NSAutoresizingMaskLayoutConstraint:0x7f931b0a6020 h=-&- v=-&- V:[NSClipView:0x7f931a489320]-(0)-|   (Names: '|':NSScrollView:0x7f931a488f10 )>",
    "<NSAutoresizingMaskLayoutConstraint:0x7f931b0a8f60 h=--& v=--& V:[NSScrollView:0x7f931a488f10(8)]>"
)

Will attempt to recover by breaking constraint 
<NSAutoresizingMaskLayoutConstraint:0x7f931b02b5c0 h=-&- v=-&- V:[MessageListView:0x7f931a489940]-(0)-|   (Names: '|':NSClipView:0x7f931a489320 )>

Set the NSUserDefault NSConstraintBasedLayoutVisualizeMutuallyExclusiveConstraints to YES to have -[NSWindow visualizeConstraints:] automatically called when this happens.  And/or, break on objc_exception_throw to catch this in the debugger.
26/01/17 12:26:35.201 PM    Vienna[270] Unable to simultaneously satisfy constraints:
(
    "<NSAutoresizingMaskLayoutConstraint:0x7f931a4ab820 h=-&- v=-&- V:|-(17)-[NSClipView:0x7f931a489320]   (Names: '|':NSScrollView:0x7f931a488f10 )>",
    "<NSAutoresizingMaskLayoutConstraint:0x7f931b0a6020 h=-&- v=-&- V:[NSClipView:0x7f931a489320]-(0)-|   (Names: '|':NSScrollView:0x7f931a488f10 )>",
    "<NSAutoresizingMaskLayoutConstraint:0x7f931b0a8f60 h=--& v=--& V:[NSScrollView:0x7f931a488f10(8)]>"
)

Will attempt to recover by breaking constraint 
<NSAutoresizingMaskLayoutConstraint:0x7f931b0a6020 h=-&- v=-&- V:[NSClipView:0x7f931a489320]-(0)-|   (Names: '|':NSScrollView:0x7f931a488f10 )>

Set the NSUserDefault NSConstraintBasedLayoutVisualizeMutuallyExclusiveConstraints to YES to have -[NSWindow visualizeConstraints:] automatically called when this happens.  And/or, break on objc_exception_throw to catch this in the debugger.

This results in the list of articles being collapsed up towards the filter bar because the splitview doesn't restore the autosaved layout correctly.

[josh64x2]

You can look at my branch mtnlion-autolayout if you want to try.

[josh64x2]

Actually maybe we should have HorizontalArticleListView and VerticalArticleListView as two different subviews so we dont have to programmatically change the layout of everything.... what do you think?

[Eitot]

I am a bit puzzled as to why the NSClipView has an autoresizing-mask constraint of 17 at the top and the NSScollView a height constraint of 8. Logically, the first should be 0, the other should not exist. It seems that it then chooses to break the wrong constraints: the top constraint of the message list and the bottom constraint of the NSClipView. That’s why the view is then compressed into a small height.

Is there anything helpful when you enable this:

defaults write uk.co.opencommunity.vienna2 NSConstraintBasedLayoutVisualizeMutuallyExclusiveConstraints -bool YES

I did not spot any Auto Layout issues in Mavericks’ console, so it must be something particular to ML.

[josh64x2]

Ooops I got the names for 10.8 and 10.9 mixed up :D I'm just going to rename my branch...

[Eitot]

I pushed a commit to my autolayout branch that hopefully fixes the tab bar. Can you merge and test this on 10.8?

[josh64x2]

Sure, I'll have to do it on Friday though after work

[josh64x2]

@Eitot the logic works, but the animation is still weird...

[josh64x2]

with regards to my mtnlion-autolayout branch, i enabled defaults write uk.co.opencommunity.vienna2 NSConstraintBasedLayoutVisualizeMutuallyExclusiveConstraints -bool YES

And it shows:

And in the console it says:

Which is weird because I didn't get a repeat of the NSAutoresizingMaskLayoutConstraint stuff but you can see the article/message list is collapsed upwards towards the filter bar.

[Eitot]

OK, I approached the tab bar differently now. Can you merge again?

[barijaona]

Don't forget that the filter bar can appear only on the main tab, not on browser tabs.

[Eitot]

I see what you mean. It is not dismissed when switching to an empty tab.

[barijaona]

Yeah, some code is needed in AppController's -handleTabChange: (or on -updateStatusBarFilterButtonVisibility)

[barijaona]

Can you have a look at my autolayout branch ?

[josh64x2]

OK on 10.8 the animation is fixed, also the filter bar now only shows on the articles tab and not the browser tabs. I like the fact that the filter shows below the tab bar - it reflects that the filter is only for the articles view and not the parent view.

Still having constraint issues, and the articles list is collapsing when turning tabs on and off as show:

and then when I expand it manually:

[barijaona]

Can you confirm if the issue with article list collapsing is specific to 10.8 ?

[josh64x2]

Well I've only tested on 10.8 and 10.12. If someone can test on 10.9 and it works I guess we could drop 10.8 support earlier.

[Eitot]

I haven’t seen this behaviour on my 10.9 VM at least. I will test the latest commits on it to be sure.

Jep, works as expected on Mavericks. On ML I see that both split views are not working correctly, they are collapsed to their smallest size. There are also a bunch of AL issues in Console.

Edit 1: I came cross this part in the AppKit release notes for 10.8:

In 10.8, NSSplitView properly respects constraints applied to its subviews, such as their minimum view widths. [...] In order to take advantage of these improvements, you must NOT implement any of the following NSSplitViewDelegate methods: splitView:constrainMinCoordinate:ofSubviewAt: splitView:constrainMaxCoordinate:ofSubviewAt: splitView:resizeSubviewsWithOldSize: splitView:shouldAdjustSizeOfSubview:

The first two methods are currently implemented. Perhaps we should try it without them.

Edit 2: Yep, removing those methods fixes this, but breaks the layout switching. The sidebar still doesn’t work. I guess we may have to implement two separate split views after all.

[barijaona]

Apart from 10.8 support, do you see other advantages in having two separate split views ? My opinion is that currently, we should prioritize code modernisation over legacy support. We can drop 10.8 compatibility early if maintaining it requires too much effort.

[josh64x2]

In my opinion it's a little cleaner having two separate NSSplitViews - they are technically different views. They would both contain the same subviews - ArticleListView (or is it MessageListView?) and ArticleView. The subviews would resize appropriately due to autolayout.

What would be a good name for the parent view of the right hand side of the app? SubscriptionDetailView? Technically it shows the detail of a subscription/folder selected on the left hand side...

[Eitot]

ContentDetailView? MainDetailView? Or just DetailView?

I just can’t seem to figure out what the problem is with the sidebar. 😣

[barijaona]

I am OK with the proposed approach and would go with FolderDetailView. Each FolderDetailView would contain a MessageListView and an ArticleView.

UnifiedDisplayView could later also benefit from this approach.

[josh64x2]

I finder Folder confusing because individual subscriptions are not folders, and we use folders for a different purpose in Vienna...

@eitot what is the issue with the side bar?

[barijaona]

I agree it might be misleading at first, but it's Vienna's terminology, either in user interface or in code (ex: FolderView), so I prefer consistency.

[josh64x2]

I am quite happy to change all references from Folder to Subscription :) - we decided on Subscription and Post in the ViennaNG discussions. Otherwise I think just DetailView.

[Eitot]

what is the issue with the side bar?

It is not sized correctly, same problem as with the other split view.

[barijaona]

Vienna's terminology is in fact quite logical once you understand that the model is Apple's Mail : in Mail, you've got the Mailbox and Message menus ; in Vienna, it's Folder and Message 🤓

[josh64x2]

Mailbox/Message is also the same as Subscription/Message then.

I'm sorry I just think it is very confusing especially considering we also have objects called Folder and Smart Folder which have nothing to do with the Folder (Subscription) and I think it could also confuse and deter potential contributors.

We can worry about it later though, and I'll call the parent view DetailView as per @Eitot's suggestion 😄

---

@Eitot I never had issues with the sidebar being sized incorrectly, even on 10.8 :( I thought I had it working well when I removed the delegate from that SplitView.

[Eitot]

Technically, Mail does not have the equivalent of a folder. There are just mailboxes. A feed/subscription would be a mailbox, an entry/article would be a message. Mail adds the additional confusion with a ‘smart mailbox’.

[barijaona]

In fact, the error was to put the "New Subscription…", "New Smart Folder…" and "New Group Folder…" commands in the "File" menu. Logically, they should be in the Folder menu to mimic Apple Mail's behavior 🤓

[Eitot]

Maybe I can have a look at the nomenclature Apple used in Snow Leopard. 😅 They did have RSS support after all.

[josh64x2]

I found this image haha

But we are losing focus on the real issue here ;) (which is the NSSplitView resizing going all weird!)

[Eitot]

Yep, in Snow Leopard’s Mail there are feeds. Feeds are added; not subscribed to. There is no consistency with regard to the name for an entry, both ‘Message’ and ‘RSS Article’ are used. There is no equivalent for a folder, but ’New Mailbox’ is indeed in a separate menu.

[josh64x2]

My preference is for consistency with the what the user sees.

If I am a user and I notice a bug with the way emoji are displayed in the title of an RSS/Atom feed that I want to try and fix, I'm going to look for "feed" references in the code, not folder. A folder is a collection of feeds for the user.

That's where I am coming from 😄

Anyway, @Eitot do you have a screenshot of the sidebar issue you're having? Also you might be interested in this very basic Swift version of Vienna I started but never got around to finishing... https://gitlab.com/ViennaRSS/viennang

[Eitot]

I find the word ‘Subscription’ a bit weird too. ‘Feed’ seems more to the point in a feed reader.

The first start of the program seems to work fine, but once you restart, the sidebar is collapsed either to the left or right as far as it can (there is a minimum-width constraint there that prevents it from collapsing all the way). I haven’t figured out what is causing it.

[josh64x2]

Hmm that sounds like the NSSplitView autosave and restore isn't working properly again :(

[Eitot]

Is everything else working?

[josh64x2]

For me it is all working on 10.12. Are there any constraint errors in the console?

[Eitot]

I meant in ML. :)

[josh64x2]

Ahh yup! Everything else works, even the sidebar doesnt collapse. it's only the message list that collapses...

[Eitot]

Curious that you can’t reproduce it. Have you perhaps enabled the state-restoration feature in ML? System Preferences → General.

[josh64x2]

I don't think so...

[josh64x2]

@Eitot I removed all of the NSSplitViewDelegate stuff from ArticleListView in my mtnlion-autolayout branch which has improved things on 10.8

I also investigated the console errors I was getting before and I believe the culprit is this guy:

I removed some of them from some views in MainMenu.xib and it reduced the number of NSAutoresizingMaskLayoutConstraint errors in the console.

A quick google also resulted in this advice:

8. Don’t Forget To Disable Autoresizing Masks For backwards compatibility of your code, the springs and struts model is still the default. For every programmatically created view that you want to use Auto Layout with, call setTranslatesAutoresizingMaskIntoConstraints:NO.

That all looks to me like it might be the root cause. We need to work out the most logical way to remove the autoresizing masks from our views and update our auto-layout constraints with the appropriate values.

[josh64x2]

So for ArticleDisplayView in Interface Builder, the attributes inspector shows this:

I think the key for mountain lion is to make the view work with that checkbox (Translates Autoresizing Mask Into Constraints) disabled (which currently doesn't work in Sierra either).

Hopefully I'll have some time tonight to try and get it working in Sierra.

[Eitot]

Disabling that option means that we have to create the constraints ourselves. I suppose we have to do this in code, at the point where the view is added to its superview.

[josh64x2]

I'm still trying to fix this 😬 Unfortunately I haven't had as much time as I would have liked lately. I hope to get it done this weekend!

[josh64x2]

@Eitot do you think sticking it in an NSBox might help?

[Eitot]

NSBox is just a subclass of NSView, I doubt that it changes anything. I actually got rid of all the boxes, because they were difficult to work with when I enabled Auto Layout.

[Eitot]

Did you have a chance to have a look at it?

[josh64x2]

Unfortunately no, I've been unwell 🤒 One of the problems seems to be related to disclosure view: for some reason the height of the tab bar gets calculated incorrectly and when it is collapsed it thinks the constraint for the height is -2.