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

Feature menu bar application implementation #123

Merged
merged 3 commits into from
Oct 23, 2020

Conversation

zenangst
Copy link
Owner

@zenangst zenangst commented Oct 23, 2020

  • Add menu bar controller for adding menu bar items to the new
    application icon that appears in the system menu bar
  • Refactor AppDelegate to clean up some of the setup code
  • Move main window setup to FeatureFactory
  • Rename MainWindow implementation to Window in order to reuse the
    implementation later on
  • Add new icons for active and inactive menu bar item

Screenshot

image

- Add menu bar controller for adding menu bar items to the new
  application icon that appears in the system menu bar
- Refactor AppDelegate to clean up some of the setup code
- Move main window setup to `FeatureFactory`
- Rename `MainWindow` implementation to `Window` in order to reuse the
  implementation later on
- Add new icons for active and inactive menu bar item
Comment on lines +59 to +64
return (applicationProvider: applicationProvider,
commandFeature: commandsController,
groupsFeature: groupFeatureController,
keyboardFeature: keyboardFeatureController,
searchFeature: searchFeatureController,
workflowFeature: workflowFeatureController)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you need to name the values when returning, as the return value already does that. Makes it a bit less verbose at least 🤷

(applicationProvider,
 commandsController,
 groupFeatureController,
 keyboardFeatureController,
 searchFeatureController,
 workflowFeatureController)

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added labels in order to make it easier to work with.
If you look at how it's used in AppDelegate, it looks like this:

context.groupsFeature.delegate = self

If we omit the labels, it would either end up being .0 (depending on the index from the tuple) or having to assign variables like let (_, groupsFeature, _, _, _) = …

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the labels should not be omitted on the return type, but when returning the tuple, I'm fairly certain we don't need to actually label them, they will be labelled automagically! (But I might be wrong)

App/Sources/Factories/FeatureFactory.swift Outdated Show resolved Hide resolved
@@ -3,6 +3,13 @@ import LogicFramework
import ViewKit
import ModelKit

typealias ApplicationStackContext = (applicationProvider: ApplicationsProvider,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this type alias is not used anywhere else than in feature factory, I say let's move inside that class

App/Sources/Menubar/MenubarController.swift Outdated Show resolved Hide resolved
ViewKit/Sources/Views/MainWindow/MainWindow.swift Outdated Show resolved Hide resolved
- Make class `final`
- Remove `ApplicationStackContext` typealias
- Add `applications` to `GroupsFeatureController.init`
- Make `MenubarController` final
@zenangst zenangst merged commit dde2ab9 into main Oct 23, 2020
@zenangst zenangst deleted the feature/menubar-implementation-part-2 branch October 23, 2020 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants