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

[meta] User Interface refactoring/enhancements #764

Closed
4 of 8 tasks
josh64x2 opened this issue Jan 17, 2017 · 27 comments
Closed
4 of 8 tasks

[meta] User Interface refactoring/enhancements #764

josh64x2 opened this issue Jan 17, 2017 · 27 comments
Labels
discussion 💬 The issue is or needs further discussion. help wanted 🆘 stale ⏳ The issue will be closed 60 days after the label was added and no inactivity has occurred since.
Milestone

Comments

@josh64x2
Copy link
Member

josh64x2 commented Jan 17, 2017

This is a meta issue to discuss/record tasks for UI refactoring and enhancements on the back of #744 and the discussion in the associated PR #761

@josh64x2
Copy link
Member Author

FYI @Eitot @barijaona

@Eitot
Copy link
Contributor

Eitot commented Jan 23, 2017

How do you want to approach the refactoring of the toolbar? Keep the toolbar-related code in AppController or refactor into a subclass (e.g. NSToolbar or NSWindowController)?

@josh64x2
Copy link
Member Author

Ok my preferred approach for all enhancements is to move as much as possible out of AppController.

I was thinking of creating an VNAToolbarController, maybe subclass from NSObject and implement NSToolbarDelegate.
Configure as much as possible using interface builder and then let the VNAToolbarController do the rest.

What do you think?

@Eitot
Copy link
Contributor

Eitot commented Jan 23, 2017

That would be my preferred approach. We could put the delegate methods and IB actions into it.

One problem is that the AppController keeps access to some resources to itself. For instance, PluginManager only lives as an instance variable in AppController, but access to it is needed by some toolbar actions as well (the style menu). Not sure how this can be done elegantly.

@josh64x2
Copy link
Member Author

OK I hate singletons but I think we could move PluginManager to a singleton which should make things easier.

@Eitot
Copy link
Contributor

Eitot commented Jan 23, 2017

Which design pattern do you prefer?

@josh64x2
Copy link
Member Author

I prefer as close to functional programming as possible but I know it isn't always possible and Vienna is basically the opposite to functional programming 😅

I am happy to have a simple and modular design that would be simple for unit and UI testing.

What do you prefer?

@Eitot
Copy link
Contributor

Eitot commented Jan 23, 2017

I am not an experienced programmer. I only got into programming through Swift. I have yet to learn how to program coherently and work with larger code bases. I do like modularity and the MVC pattern, but then I start refactoring things every so often, defeating the purpose of modularity. 😀

Objective-C is a different beast to me and I have no idea how to approach it yet.

@josh64x2
Copy link
Member Author

Well you seem experienced!

Once we drop 10.8 we can start using Swift more which will be good :)

@Eitot
Copy link
Contributor

Eitot commented Jan 24, 2017

Mavericks solves everything 😜; Swift, stack views, NSURLSession, full base localisation, improvements to asset catalogs and a bunch of other useful Cocoa APIs.

@josh64x2
Copy link
Member Author

OK I'll try to fix some of the remaining 10.8 bugs in the autolayout branch, and after we release it we can drop 10.8 support unless it is for bug fixes which I can back port.

If we are spending too much time fixing autolayout bugs for 10.8 then I suggest we end 10.8 support early.

@Eitot
Copy link
Contributor

Eitot commented Jan 25, 2017

The DisclosureView still has some bugs. It still happens that a constraint is added twice. I have made small changes to it to alleviate it somewhat, but I don’t know how I can prevent it effectively. There is no way to disable a constraint directly <10.11. It may also be possible to just set the constraint in IB and don’t remove it, but that means that the height cannot change dynamically.

I think I also spotted a concurrency bug that can happen when the filter bar should collapse. I think the direct cause is this line here in -setFilterBarState:withAnimation: in AppController.m:

[self searchUsingFilterField:self];

Eventually, this line will result in -getArticlesWithCompletionBlock in ArticleController.m being called, which executes -filterString in AppController.m on a different thread.

I am not sure if this has something to do with the implicit animation in DisclosureView, because I have been seeing these messages in the debugger which are similarly pointing to these last two methods:

CoreAnimation: warning, deleted thread with uncommitted CATransaction; set CA_DEBUG_TRANSACTIONS=1 in environment to log backtraces.

Here an image of the stack trace:
screen shot 2017-01-25 at 06 40 53

@josh64x2
Copy link
Member Author

Interesting discovery @Eitot ! I'll see what I can do about it.

@Eitot
Copy link
Contributor

Eitot commented Jan 26, 2017

The smart-folder panel also needs some work. It is probably better to put it into a table view. Stack view would also do.

@josh64x2
Copy link
Member Author

Stack view isn't available on 10.8 is it? Or is this something that would require more work and should be >= 10.9-only ?

@Eitot
Copy link
Contributor

Eitot commented Jan 26, 2017

Yep, 10.9 only. It currently just adds the subviews to the hierarchy, but I reckon that this is not compatible with Auto Layout, hence why it doesn’t work correctly yet.

@Eitot
Copy link
Contributor

Eitot commented Feb 6, 2017

I have made a start on the toolbar in IB. Unfortunately, I have been running into lots of wee problems.

The toolbar depends on the plugin manager (for the custom toolbar items), but the plugin manager itself is only instantiated in -awakeFromNib:, so after the toolbar is loaded. This means that the toolbar calls its delegate before the plugins are initialised. I thought about just instantiating the plugin manager in AppController’s -init:, but it turns out that the plugin manager cross-references the main menu again for the menu items and accordingly fails, because the menu isn’t there yet.

Am I overlooking something? 😔

@josh64x2
Copy link
Member Author

josh64x2 commented Feb 6, 2017

It is recommended practice to keep the MainMenu.xib separate from the NSWindow xibs of your app. If we do that, MainMenu can load with PluginManager and then we can instantiate our main window + NSWindowController.

Would this approach work? It is something we need to do sooner or later so I could break out NSWindow + NSWindowController.

I really want to try and get the autolayout working in 10.8 first though.

@Eitot
Copy link
Contributor

Eitot commented Feb 7, 2017

No worries, this is just something I work on. What needs to be done before you can merge your branch into autolayout?

I actually think that Plugin Manager should not have any code that directly relies upon the UI. That’s a violation of the MVC pattern. It should do its tasks and then have a controller request the necessary information to build the UI.

@josh64x2
Copy link
Member Author

josh64x2 commented Feb 7, 2017

I need remove the auto resizing masks and use constraints for the splitview i think. I was hoping to do it tonight but unfortunately something else came up.

I agree with you about Plugin Manager. I guess the other thing to consider is it worth making these changes now or should it wait for Swift?

@Eitot
Copy link
Contributor

Eitot commented Feb 7, 2017

That depends on how strongly we are committed to Mountain Lion.

Ideally, I would like to rewrite parts of the plugin manager and take advantage of Apple’s NSSharingService class for native sharing support and the extensions in Yosemite. Technically, this can be supported just fine in Mountain Lion, but I am not experienced with Objective-C. I’d prefer Swift, but I am not unwilling to learn more about Objective-C. I’m a bit worried that I have been spoilt by Swift and will try to make Swift concepts work in Objective-C. 😂

@josh64x2
Copy link
Member Author

josh64x2 commented Feb 7, 2017

Well, so far your Objective-C code looks great! And if you send me an email to my gmail account (josh64@) I can send you some Objective-C resources that might help.

I do feel bad dropping 10.8 support, but now that is 4 versions behind, and any system that can run 10.8 can also run 10.9 - 10.11 I think we should drop it as soon as I can fix autolayout.
I will create a new branch after that for mountain-lion which is where the last version that supported mountain lion can live. We can back-port any bug fixes that are easy enough or a new volunteer that needs 10.8 support can take over maintenance of that branch.

I know some people have a good reason for staying on 10.9 in the poll I ran a couple of months ago on the forums so I wouldn't mind supporting 10.9 for a bit longer, and then maybe move to 10.11 as the minimum after the new version of macOS is announced at WWDC.

@Eitot Eitot added discussion 💬 The issue is or needs further discussion. and removed developer request 🙏 labels Feb 8, 2017
@Eitot
Copy link
Contributor

Eitot commented Feb 8, 2017

Sounds reasonable. At the end of the day, it doesn’t really help us if we constrain ourselves when there is still so much that can be done. Apple seems to be ramping up the development of Cocoa with lots of convenience classes and UIKit transplants, which would be unfortunate to miss.

@josh64x2
Copy link
Member Author

Well I've tried a lot of things to fix it - you think it would be simple to add some constraints to pin the top/bottom/left/right to the superview, but it's not because Interface Builder doesn't recognise that the superview is the "Partner View".

I'm thinking NSViewController and add the constraints in code using the visual format language.

@Eitot Eitot mentioned this issue Mar 6, 2017
@Eitot
Copy link
Contributor

Eitot commented Mar 9, 2017

Are we ready to update the project config for 10.9?

@josh64x2
Copy link
Member Author

How are we tracking on these tasks? :P

@stale
Copy link

stale bot commented Sep 28, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale ⏳ The issue will be closed 60 days after the label was added and no inactivity has occurred since. label Sep 28, 2020
@stale stale bot closed this as completed Oct 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion 💬 The issue is or needs further discussion. help wanted 🆘 stale ⏳ The issue will be closed 60 days after the label was added and no inactivity has occurred since.
Projects
None yet
Development

No branches or pull requests

3 participants