iMediaSandboxing / iMedia

The Karelia iMedia Browser, framework and application for browsing media on Mac OS X
http://www.karelia.com/imedia/
Other
18 stars 12 forks source link

Switch to NSCollectionView from IKImageBrowserView #100

Open danielpunkass opened 6 years ago

danielpunkass commented 6 years ago

In the latest Xcode 9.3, warnings are issued about the deprecated use of IKImageBrowserView by iMedia.

Looking forward, it seems that switching to NSCollectionView is the best long-term strategy, both to alleviate this deprecated dependency, but also probably to improve the performance and overall functionality of the icon view in iMedia.

This is likely to be a disruptive change. I started down an exploratory path of changing IMBImageBrowserView from a subclass of IKImageBrowserView to a subclass of NSCollectionView. A lot of supporting code becomes unncessary, and some new accommodations must be made. Generally speaking the appearance can be approximately preserved but some details would be hard to replicate in with the new API.

Because NSCollectionView was dramatically redesigned for masOS 10.11 (https://developer.apple.com/library/content/releasenotes/AppKit/RN-AppKitOlderNotes/index.html#10_11CollectionView), and further enhanced for 10.12, a switch to NSCollectionView will either require increasing the minimum system requirement for iMedia, or putting considerable work into developing a dual strategy approach for continuing to use IKImageBrowserView on 10.10 and using NSCollectionView on later OS releases.

Because I am currently focused on 10.12+ for my own projects, the thrust of my experiments have been on the assumption that I would require 10.12.

I will be happy to share my work-in-progress if and when any other folks are interested to take this forward as an option for iMedia. A reasonable approach might be to maintain a collection-view branch that tracks the development branch, so that while existing iMedia clients want to maintain their current OS requirements, they can stick with the older IKImageBrowserView based approach.

iljaiwas commented 6 years ago

I'd like to see the NSCollectionView implementation becoming the default one. Let's burry the IKImageBrowserView approach on some maintenance-only branch.

What's the minimum system requirement for your ifolor app right now, @jjac ?

jjac commented 6 years ago

@iljaiwas We still support macOS 10.10

danielpunkass commented 6 years ago

I think a maintenance branch for 10.10 makes sense, especially considering how little activity over all there has been in the project for the past several years. Unless things really pick up it should be reasonably easy to keep a maintenance branch in sync with a forward-moving development branch for folks who are ready to require later OS releases. @jjac would you have any reservations about switching your 10.10 app to building against a specific branch for 10.10 support?

jjac commented 6 years ago

I.e. you would want to support 10.11 NSCollectionView within image browser on development branch?

danielpunkass commented 6 years ago

That's what I would imagine, or possibly 10.12 given there were further enhancements to NSCollectionView on that release. I think that if we were to make a change in the minimum required system, it might as well be a little more aggressive than bumping by one version.

I would also be totally fine with putting 10.12+ support on a branch. Or even just have a specific "collection view" branch that happens to require 10.12+.

danielpunkass commented 5 years ago

@gloubibou Maybe we should move conversation about NSCollectionView here. (Re: https://github.com/iMediaSandboxing/iMedia/issues/109#issuecomment-462766550)

peterb180369 commented 5 years ago

Hi Daniel,

While FotoMagico 5 still has a deployment target of 10.10, I’d be willing to switch to 10.12 should new point releases with bugfixes become necessary. FotoMagico 6 will have a deployment target of either 10.12 or 10.13, so we are good here.

Peter

On 11. Apr 2018, at 15:05, Daniel Jalkut notifications@github.com wrote:

In the latest Xcode 9.3, warnings are issued about the deprecated use of IKImageBrowserView by iMedia.

Looking forward, it seems that switching to NSCollectionView is the best long-term strategy, both to alleviate this deprecated dependency, but also probably to improve the performance and overall functionality of the icon view in iMedia.

This is likely to be a disruptive change. I started down an exploratory path of changing IMBImageBrowserView from a subclass of IKImageBrowserView to a subclass of NSCollectionView. A lot of supporting code becomes unncessary, and some new accommodations must be made. Generally speaking the appearance can be approximately preserved but some details would be hard to replicate in with the new API.

Because NSCollectionView was dramatically redesigned for masOS 10.11 (https://developer.apple.com/library/content/releasenotes/AppKit/RN-AppKitOlderNotes/index.html#10_11CollectionView https://developer.apple.com/library/content/releasenotes/AppKit/RN-AppKitOlderNotes/index.html#10_11CollectionView), and further enhanced for 10.12, a switch to NSCollectionView will either require increasing the minimum system requirement for iMedia, or putting considerable work into developing a dual strategy approach for continuing to use IKImageBrowserView on 10.10 and using NSCollectionView on later OS releases.

Because I am currently focused on 10.12+ for my own projects, the thrust of my experiments have been on the assumption that I would require 10.12.

I will be happy to share my work-in-progress if and when any other folks are interested to take this forward as an option for iMedia. A reasonable approach might be to maintain a collection-view branch that tracks the development branch, so that while existing iMedia clients want to maintain their current OS requirements, they can stick with the older IKImageBrowserView based approach.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/iMediaSandboxing/iMedia/issues/100, or mute the thread https://github.com/notifications/unsubscribe-auth/AAu5O1SCXju_DraDs_6-cUuRhBz-5pbpks5tnf-2gaJpZM4TP_b7.

danielpunkass commented 5 years ago

@peterb180369 Thanks. I am going to move ahead with a branch for NSCollectionView support that assumes 10.12 or greater support. If anybody wants to track my progress, I'm working on a branch here: https://github.com/danielpunkass/iMedia/tree/collection-view-revamp

It's fairly far long and I'm chipping away at problems. I'm pretty much only working on this this week so I hope to have something pretty robust done soon.

jjac commented 5 years ago

@danielpunkass Unfortunately, we will have to support 10.11 probably until Feb. 2020. Are there substantial benefits to the NSCollectionView API / maturity with respect to 10.12 or is it just that MarsEdit's deployment target is 10.12?

danielpunkass commented 5 years ago

@jjac I think the bulk of NSCollectionView fixes that we "need" for iMedia were made in 10.11, but because it was substantially improved for 10.12 as well, I don't know if there are subtle problems in deploying against 10.11. The release notes for 10.12 list a bunch of enhancements but they seem mostly unrelated to at least the way I've been using it thus var in my branch: https://developer.apple.com/library/archive/releasenotes/AppKit/RN-AppKitOlderNotes/index.html

Because I'm on a kind of fast-track to getting something working for MarsEdit, which does require 10.12, I'm going to continue focusing on 10.12 support but I think before merging my branch anyway, other affected people should test against it to see how it works in their apps. Maybe as part of that you could experiment with requiring only 10.11 and see how it goes?

BTW there are numerous Dark Mode problems in iMedia and among them, the background color of IKImageBrowser not changing seems to be a problem that is annoying to work around. I am glad to say that at least that issue seems to be addressed by switching to NSCollectionView.

jjac commented 5 years ago

@danielpunkass sounds like a good plan.

We still utilize a bunch of proprietary controls within our macOS app preventing us from short-term supporting dark mode but advancing iMedia towards dark mode support will certainly help.

jjac commented 5 years ago

Has anyone of you already looked into IKImageBrowser behavior on 10.15 (apart from deprecation)?

@danielpunkass you seem to have made the transition within your fork? Could this be harvested into the master repo?

jjac commented 5 years ago

Ok, I built the tester app from Xcode 11, ran it on 10.15 (Beta 5) and IKImageBrowser does not seem to be an issue.

danielpunkass commented 5 years ago

@jjac It's probably a good idea for the master repo to switch to NSCollectionView at some point, to avoid being surprised when/if IKImageBrowser suddenly stops working correctly one day. It's been a long time since I made my changes, I'll take a fresh look and see if I can create a pull request as a basis for consideration.

jjac commented 5 years ago

@danielpunkass thanks! I noticed, that in MarsEdit multi-selection of images does not seem to be supported. Is that a restriction of your migration to NSCollectionView or solely a restriction within your app?

danielpunkass commented 5 years ago

@jjac It's probably just a restriction of my app (I don't handle multiple insertion of images, so I don't allow selecting them). However, there are undoubtedly some areas where my work would need to be expanded on for the benefit of the greater project. I disable/hide lots of iMedias's UI so there might be some behaviors that would need to be updated.