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

Monospace menubar #685

Closed
wants to merge 6 commits into from
Closed

Conversation

frostplexx
Copy link

This PR addresses issue #56 by introducing three new options:

# Font size for menu bar
font-size = 12.0

# Font family for the menu bar items
# Use the Font Book to find fonts
font-family = 'JetBrainsMono Nerd Font Mono'

# Possible values: icon|horizontal_full|horizontal_min
# icon: shows only the current workspace
# horizontal_full: shows all workspaces, with the current one highlighted
# horizontal_min: shows all workspaces with open windows,and the current one
workspace-indicator-style = 'icon'

To accomplish this, I had to rewrite all menu bar items to use AppKit, as pure SwiftUI lacks the capability to display arbitrary views.

Here are some pictures demonstrating the feature in action:
image
image
image
image

@cvlvxi
Copy link

cvlvxi commented Nov 11, 2024

Can we get a video demo? This seems pretty interesting

@frostplexx
Copy link
Author

Screen.Recording.2024-11-11.at.1.29.11.PM.mov

Emojis are pushed to the end, which is the same behavior that exists.

Copy link
Owner

@nikitabobko nikitabobko left a comment

Choose a reason for hiding this comment

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

Thanks for your work, but I am not ready to accept such big PR. The PR is hardly reviewerable.

Please, fix problem by problem in small incremental accurate commits. The good first step is to only make the indicator monospaced and nothing else. Everything else can be discussed after that.

import Foundation
import SwiftUI

class AppDelegate: NSObject, NSApplicationDelegate {
Copy link
Owner

Choose a reason for hiding this comment

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

Sources/AeroSpaceApp should stay as small as possible, because this "module" is shared between Xcode and SPM. All the code must be pushed to Sources/AppBundle module

Comment on lines +135 to +157
// Add enable/disable item
let enableItem = NSMenuItem(
title: viewModel.isEnabled ? "Disable" : "Enable", action: #selector(toggleEnabled(_:)), keyEquivalent: "e")
enableItem.target = self
menu.addItem(enableItem)

// Add open config item
let editor = getTextEditorToOpenConfig()
let openConfigItem = NSMenuItem(
title: "Open config in '\(editor.lastPathComponent)'", action: #selector(openConfig(_:)), keyEquivalent: "o"
)
openConfigItem.target = self
menu.addItem(openConfigItem)

// Add reload config item if enabled
if viewModel.isEnabled {
let reloadConfigItem = NSMenuItem(
title: "Reload config", action: #selector(reloadConfigAction(_:)), keyEquivalent: "r")
reloadConfigItem.target = self
menu.addItem(reloadConfigItem)
}

// Add quit item
Copy link
Owner

Choose a reason for hiding this comment

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

Please avoid obvious comments

Comment on lines +46 to +51
# Font size for menu bar
font-size = 12.0

# Font family for the menu bar items
# Use the Font Book to find fonts
font-family = 'Andale Mono'
Copy link
Owner

Choose a reason for hiding this comment

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

I don't think that font should be configurable. We should just pick a good default and stick with it.

@nikitabobko nikitabobko added the pr-rejected Pull Request is rejected label Nov 14, 2024
@nikitabobko
Copy link
Owner

Also see: #711 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-rejected Pull Request is rejected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants