Feature/dynamic menubar icon #43

asvela · 2020-04-05T14:01:11Z

Possible base for new feature for displaying the current CPU temperature rather than a static icon in the menubar (addressing feature request #42).

To do:

Option to display icon or temperature
(Option to choose CPU or GPU temperature)

Tested on macOS 10.14.6.

(Let me know if I should have done this differently than opening a pull request, I am not familiar with how to contribute)

…mals in the menubar

…r 'git submdules update'

DanielStormApps · 2020-04-05T17:35:43Z

Thank you for taking a stab at this @asvela !

I'll review the PR and leave some feedback. In its current state with the TODO's I wouldn't want to merge this in. Would you be ok with me taking this feature over?

DanielStormApps · 2020-04-05T17:37:32Z

Fanny/Fanny.xcodeproj/project.pbxproj

@@ -658,15 +658,15 @@
 				CODE_SIGN_STYLE = Automatic;
 				COMBINE_HIDPI_IMAGES = YES;
 				CURRENT_PROJECT_VERSION = 1;
-				DEVELOPMENT_TEAM = E5BK5K7RRN;
+				DEVELOPMENT_TEAM = HT8VTXFQ39;


I need to look into a way to disambiguate this so its not always changing 🤔

Maybe I just should just not have committed the xcodeproj? As I did not do any other changes there than the version (I think)

That’d work, but I don’t want to have to try to enforce that on every PR.

.gitmodules

DanielStormApps · 2020-04-05T17:38:37Z

Fanny/Fanny.xcodeproj/project.pbxproj

 				ENABLE_HARDENED_RUNTIME = YES;
 				INFOPLIST_FILE = fanny/Info.plist;
 				LD_RUNPATH_SEARCH_PATHS = (
 					"$(inherited)",
 					"@executable_path/../Frameworks",
 				);
 				MACOSX_DEPLOYMENT_TARGET = 10.13;
-				MARKETING_VERSION = 2.1.0;
+				MARKETING_VERSION = 2.1.1;


We'd want to version the project separately from a PR.

Makes sense. I was not sure about the conventions here, if you bounce it upon completing a new version or when you start adding things

DanielStormApps · 2020-04-05T17:40:25Z

Fanny/fanny/Menu/FNYStatusBar.swift

@@ -19,15 +19,20 @@ class FNYStatusBar: NSStatusBar {
    override init() {
        super.init()
        statusItem = NSStatusBar.system.statusItem(withLength: NSStatusItem.variableLength)
-        applyStatusItemIcon()
+        statusItem?.button?.title = "Loading.."


Not sure if this is necessary. Can you elaborate?

I am not sure either, hehe! It does take those first x seconds of the refresh interval before the first number appears, but maybe the icon should rather be displayed during this time? At the same time this might be confusing; if you have a long refresh interval (60 sec) and change the setting governing if the temperature rather than the icon should be displayed, but then seemingly nothing changes for the first 60 secs and you wonder if the setting was really changed.

Fanny/fanny/Menu/FNYStatusBar.swift

DanielStormApps · 2020-04-05T17:43:50Z

Fanny/fanny/Menu/FNYStatusBar.swift

+
+    func updateStatusItem() {
+        let cpuTemperature = SMC.shared.cpuTemperatureAverage()
+        statusItem?.button?.title = cpuTemperature!.formattedTemperature(decimals: 0, useSpaceDelimiter: false)


Can see the force unwrap ! causing issues here.

I was not sure how this should be done, addressed it in 8461f4f with a possible fix

asvela · 2020-04-05T18:30:22Z

Thank you for taking a stab at this @asvela !

I'll review the PR and leave some feedback. In its current state with the TODO's I wouldn't want to merge this in. Would you be ok with me taking this feature over?

Sure, absolutely! I agree it is not ready to merge in quite yet, but was not sure how I would share it otherwise than making a pull request (if I should have done it differently please let me know).

As I said previously, I have no experience with this so please excuse the not so elegant way of doing it. Happy to assist if there are other things I could possibly do.

… Empty does not remove the menubar item, just makes it 'invisible'

asvela · 2020-04-06T12:57:34Z

Made some changes to add icon/temperature/nothing preference option:

However, I am not sure how to remove/hide the menubar item all together. Currently it is just an empty narrow strip:

The storyboard constraints were confusing, so they might need some revision too.

vanthuanqs · 2020-04-07T07:43:00Z

Thank guys, I was looking for this feature for a long time, hope it's going to live soon

DanielStormApps · 2020-04-07T13:21:48Z

Thank you for taking a stab at this @asvela !
I'll review the PR and leave some feedback. In its current state with the TODO's I wouldn't want to merge this in. Would you be ok with me taking this feature over?

Sure, absolutely! I agree it is not ready to merge in quite yet, but was not sure how I would share it otherwise than making a pull request (if I should have done it differently please let me know).

As I said previously, I have no experience with this so please excuse the not so elegant way of doing it. Happy to assist if there are other things I could possibly do.

A PR is the right way to share 👍

DanielStormApps · 2020-04-07T13:23:10Z

Made some changes to add icon/temperature/nothing preference option:

However, I am not sure how to remove/hide the menubar item all together. Currently it is just an empty narrow strip:

The storyboard constraints were confusing, so they might need some revision too.

Nice! Thank you!
I won’t have time until later this week to go over it all. Will keep ya posted 👍

…updates. Update preference view controller constraints. Revert version to 2.1.0.

DanielStormApps · 2020-04-10T22:12:44Z

@asvela Merged ✅

Made some changes on another branch: b352a3f

I removed the "hidden" menu bar option. This isn't something I'd like to support.
If you'd like to hide your menu bar icons take a look at Bartender: https://www.macbartender.com

v2.2.0 is ready for download and contains these new features.

Thank you for the PR! 👍

asvela · 2020-04-11T20:07:23Z

Thanks for including this!

I posted a little comment on considering removing the decimals (possibly also the space) from the menubar as they take up space and are not really adding reliable information.

Regarding

I removed the "hidden" menu bar option. This isn't something I'd like to support.

I just thought I'd mention this from Apple's Human interface guidelines:

Let people decide whether to enable your menu bar extra. Users, not apps, should choose when a menu bar extra is added to the menu bar. Typically, this is done by changing a setting in an app’s preferences window. To ensure discoverability, however, consider giving people the option of enabling the menu bar extra during setup.

It might be someone wants to use the nice widget but not the menubar icon. Relying on third party apps for hiding it is not ideal, and many programs include this feature of removing it (Dropbox, Alfred, Clipy are just some examples that came to mind). However, it seems the app needs some restructuring to do that properly, not just the cheesy way I did it, so it might be a lot of work for a niche feature.

DanielStormApps · 2020-04-12T21:59:00Z

Unfortunately there's no way around not having a menu bar icon. Today Extension widgets require that sandbox is enabled so the widget needs needs a "main" application to retrieve all the system values. The alternative would be having an application running in your dock at all times, which isn't better IMO. Setting the menu bar to an empty string with no icon will cause confusion.

asvela added 2 commits April 5, 2020 15:42

Easy implmentation of displaying the CPU temperature without any deci…

495538c

…mals in the menubar

(gitmodules) updating with https link, otherwise permission denied fo…

abf8b8f

…r 'git submdules update'

DanielStormApps reviewed Apr 5, 2020

View reviewed changes

.gitmodules Show resolved Hide resolved

DanielStormApps reviewed Apr 5, 2020

View reviewed changes

Fanny/fanny/Menu/FNYStatusBar.swift Outdated Show resolved Hide resolved

DanielStormApps reviewed Apr 5, 2020

View reviewed changes

DanielStormApps self-assigned this Apr 5, 2020

DanielStormApps added the feature request label Apr 5, 2020

DanielStormApps linked an issue Apr 5, 2020 that may be closed by this pull request

Feature request: Display CPU temperature in the statusbar #42

Closed

Added preference item for choosing to display icon/temperature/empty.…

8461f4f

… Empty does not remove the menubar item, just makes it 'invisible'

DanielStormApps added a commit that referenced this pull request Apr 10, 2020

#43 Updating naming. Add CPU/GPU menu bar options. Refactor menu bar …

b352a3f

…updates. Update preference view controller constraints. Revert version to 2.1.0.

DanielStormApps merged commit f426793 into DanielStormApps:master Apr 10, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/dynamic menubar icon #43

Feature/dynamic menubar icon #43

asvela commented Apr 5, 2020

DanielStormApps commented Apr 5, 2020

DanielStormApps Apr 5, 2020

asvela Apr 5, 2020

DanielStormApps Apr 7, 2020

DanielStormApps Apr 5, 2020

asvela Apr 5, 2020

DanielStormApps Apr 5, 2020

asvela Apr 5, 2020

DanielStormApps Apr 5, 2020

asvela Apr 6, 2020

asvela commented Apr 5, 2020

asvela commented Apr 6, 2020

vanthuanqs commented Apr 7, 2020

DanielStormApps commented Apr 7, 2020

DanielStormApps commented Apr 7, 2020

DanielStormApps commented Apr 10, 2020

asvela commented Apr 11, 2020 •

edited

DanielStormApps commented Apr 12, 2020

Feature/dynamic menubar icon #43

Feature/dynamic menubar icon #43

Conversation

asvela commented Apr 5, 2020

DanielStormApps commented Apr 5, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asvela commented Apr 5, 2020

asvela commented Apr 6, 2020

vanthuanqs commented Apr 7, 2020

DanielStormApps commented Apr 7, 2020

DanielStormApps commented Apr 7, 2020

DanielStormApps commented Apr 10, 2020

asvela commented Apr 11, 2020 • edited

DanielStormApps commented Apr 12, 2020

asvela commented Apr 11, 2020 •

edited