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

Implements autounhide when screen is wider than set width #413

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

michyprima
Copy link
Contributor

For ex bartender users like me, this is the "Show all menu bar items when active screen is bigger than" counterpart.

@jordanbaird
Copy link
Owner

Please run SwiftLint and make sure there are no warnings or errors.

@michyprima
Copy link
Contributor Author

Done! Sorry about that.

@lgarron
Copy link

lgarron commented Nov 23, 2024

I was able to build and run this locally. I can set the config successfully, but it shows the menu items below the bar, not inline (as they would be shown if Ice wasn't running). Would it be easy to change that?

@lgarron
Copy link

lgarron commented Nov 23, 2024

I was able to build and run this locally. I can set the config successfully, but it shows the menu items below the bar, not inline (as they would be shown if Ice wasn't running). Would it be easy to change that?

Ah, I think this is because I have "Use Ice Bar" enabled. This is necessary for a MacBook Pro screen with a notch to avoid menu items going under the notch, which seems to be a concern for a lot of us per the issues on this repo.

It would be useful for this setting to be contextually overridable (ideally by default) when showing all items for a wide screen.

@lgarron
Copy link

lgarron commented Nov 23, 2024

It would be useful for this setting to be contextually overridable (ideally by default) when showing all items for a wide screen.

Okay, here's an implementation of that: michyprima#1

michyprima and others added 2 commits November 23, 2024 21:06
…erriding-use-ice-bar

When showing items due to wide screen, always show inline and never hide.
@michyprima
Copy link
Contributor Author

@lgarron merged. I didn't catch that because I do not use the bar, sorry

@lgarron
Copy link

lgarron commented Nov 23, 2024

One additional issue I see is that this only listens to one screen at a time. If I switch between the built-in display and a wide monitor it works fine, but if I use both side-by-side then the built-in display uses the setting from the monitor. I'm not sure how to best manage that.

@lgarron
Copy link

lgarron commented Nov 23, 2024

Also making a note here because I don't have a better place: this makes the menu item for Ice a no-op. It should probably show its dropdown or do something to at least acknowledge it's been clicked.

(Even more intricate would be for this to trigger an additional user-controlled override. But I personally don't need this, and I suspect it also wouldn't be useful to most people who just want Ice to hide things when the screen is small.)

Looks like that would be here:

case .leftMouseDown, .leftMouseUp:
if NSEvent.modifierFlags == .control {
statusItem.showMenu(createMenu(with: appState))
} else if
NSEvent.modifierFlags == .option,
appState.settingsManager.advancedSettingsManager.canToggleAlwaysHiddenSection
{
if let alwaysHiddenSection = appState.menuBarManager.section(withName: .alwaysHidden) {
alwaysHiddenSection.toggle()
}
} else {
section?.toggle()
}

@ViewBuilder
private var activeScreenWidthToggle: some View {
Toggle("Automatically unhide when active screen width is higher than the value below", isOn: manager.bindings.showHiddenSectionWhenWidthGreaterThanEnabled)
.annotation("This will always show the items in the menu bar (ignoring the \"Use Ice Bar\" setting.")
Copy link

Choose a reason for hiding this comment

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

Looks like I forgot a closing paren here.

Suggested change
.annotation("This will always show the items in the menu bar (ignoring the \"Use Ice Bar\" setting.")
.annotation("This will always show the items in the menu bar (ignoring the \"Use Ice Bar\" setting).")

@lgarron
Copy link

lgarron commented Dec 3, 2024

Also making a note here because I don't have a better place: this makes the menu item for Ice a no-op. It should probably show its dropdown or do something to at least acknowledge it's been clicked.

For what it's worth, I've now been running a build with this PR for a week, and I've been fairly happy with it. This is the one detail I would personally still try to improve.

EDIT: Never mind, I forgot about the issue when using multiple monitors with different sizes, that would still be good to improve as well.

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.

3 participants