Skip to content
This repository has been archived by the owner on Sep 17, 2021. It is now read-only.

Implementation of system control tray option #35

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

Conversation

xiamaz
Copy link

@xiamaz xiamaz commented Dec 26, 2018

Add an touchbar option to show the system controls of the normal
touchbar.

This option adds an energybar button to the system controls, which
can be used to expand the energybar into the application space of
the touchbar.

This way it is possible to use the system provided control button and offers a quick way to show the native touchbar controls.

@ghost
Copy link

ghost commented Jan 13, 2019

I am fairly new to GitHub and was wondering if you could show me how to implement this, if implement is the right word. Thanks in advance 😊

@xiamaz
Copy link
Author

xiamaz commented Jan 13, 2019

I am fairly new to GitHub and was wondering if you could show me how to implement this, if implement is the right word. Thanks in advance 😊

@Ninjien You can refer to this example project for a very simple example: https://github.com/a2/touch-baer

Essentially you create a widget, which will be shown on the touchbar by utilizing DFRFoundation private APIs. This process is fairly limited, as you can only show one additional button in the system controls. (This is also used by programs such as Quicktime or XCode debugger)

@billziss-gh
Copy link
Owner

billziss-gh commented Jan 16, 2019

@xiamaz thanks for the PR and I apologize for not being able to check it out earlier.

Overall I like the idea and I think we should incorporate it. However I found a some usability issues that I think we have to discuss and address first.

The main problem is that it is possible to check "Show system controls" and then close the EnergyBar window. Because EnergyBar is an LSUIElement application it is not easy to open the EnergyBar window again and uncheck "Show system controls".

  • The obvious way is to rerun EnergyBar, which shows the main window again (via applicationShouldHandleReopen:hasVisibleWindows:). While this works the average user may not think of it. (I am the project's original author and I was actually puzzled for a minute on how to get back the window while testing.)

  • I like the idea of adding a button to the Control Strip to bring back the EnergyBar. There are however a couple of problems with this approach:

    • The user may not have the Control Strip enabled in System Preferences | Keyboard. For example, in my system I have this always set to "Expanded Control Strip", because this makes EnergyBar customization easiest and because I never use the App Controls.

    • Even when the Control Strip is enabled in System Preferences | Keyboard, the button displays the EnergyBar together with the Control Strip. IMO this has problems and would recommend that we simply bring the EnergyBar back "on top" in this case.

  • I have not researched why, but it is possible to get 2 esc buttons with this PR.

PS: I only did some usability testing and have not reviewed the code. However I noticed in passing what appears to be a problem and will add a review comment after this message

- (id)init
{
if (self = [super init]) {
_button = [[[NSCustomTouchBarItem alloc] initWithIdentifier:kControlButtonIdentifier] autorelease];
Copy link
Owner

Choose a reason for hiding this comment

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

This project does not use ARC, so this code will autorelease the _button, which could end up as a pointer to free memory.

Also the _button should be freed in dealloc.

@xiamaz
Copy link
Author

xiamaz commented Feb 24, 2019

@xiamaz thanks for the PR and I apologize for not being able to check it out earlier.

Thanks for the reply. I want to continue working on the PR, if there are good ways to overcome the usability issues.

Overall I like the idea and I think we should incorporate it. However I found a some usability issues that I think we have to discuss and address first.

The main problem is that it is possible to check "Show system controls" and then close the EnergyBar window. Because EnergyBar is an LSUIElement application it is not easy to open the EnergyBar window again and uncheck "Show system controls".

* The obvious way is to rerun EnergyBar, which shows the main window again (via `applicationShouldHandleReopen:hasVisibleWindows:`). While this works the average user may not think of it. (I am the project's original author and I was actually puzzled for a minute on how to get back the window while testing.)

Yes, I always just relaunched the EnergyBar app. If this could be blocked by some settings. I would suggest adding the ability to show Energybar settings by long-pressing the control tray button.

Still the current setting of long-pressing the date seems also to be an issue, if the user is not enabling this specific widget.

* I like the idea of adding a button to the Control Strip to bring back the EnergyBar. There are however a couple of problems with this approach:
  
  * The user may not have the Control Strip enabled in System Preferences | Keyboard. For example, in my system I have this always set to "Expanded Control Strip", because this makes EnergyBar customization easiest and because I never use the App Controls.

I only implemented it because I prefer the system controls, since otherwise we would need to reimplement all system control functionality in energybar, eg a IME switcher is currently missing.

  * Even when the Control Strip is enabled in System Preferences | Keyboard, the button displays the EnergyBar together with the Control Strip. IMO this has problems and would recommend that we simply bring the EnergyBar back "on top" in this case.

I would like to keep both, since I like having access to system controls directly. Maybe we should add this as a option?

* I have not researched why, but it is possible to get 2 esc buttons with this PR.

This probably because you had an esc widget enabled. If you are in the Energybar app the outer close button might be shown as another esc button.

PS: I only did some usability testing and have not reviewed the code. However I noticed in passing what appears to be a problem and will add a review comment after this message

I will update that.

Add an touchbar option to show the system controls of the normal
touchbar.

This option adds an energybar button to the system controls, which
can be used to expand the energybar into the application space of
the touchbar.
Refactoring of control strip button code into TouchController, avoiding
additional imports in the AppController.

Now expanding the system modal will also hide the energybar button itself
by passing the correct identifier to the present function.
@xiamaz
Copy link
Author

xiamaz commented May 30, 2019

I have rebased against the current version. Are you still considering this PR or have some suggestions?

Repository owner deleted a comment from jacobeatsspam Oct 24, 2019
Repository owner deleted a comment from jacobeatsspam Oct 24, 2019
Repository owner deleted a comment from jacobeatsspam Oct 24, 2019
Repository owner deleted a comment from jacobeatsspam Oct 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants