[Control Pad] Refactor app for consistency and reliability, as well as UI updates #4135
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,11 @@ | ||
| 0.01: New app - forked from widhid | ||
| 0.02: Minor code improvements | ||
| 0.03: Major code refactor | ||
| Change to a rounded style | ||
| Add a button for settings | ||
| Make buttons/toggles much more consistent and reliable | ||
| Instead of text, use icons instead | ||
| Make menu slightly wider to feel more spacious without losing the overlay feel | ||
| Change toggle styles/colors slightly | ||
| Migrate DnD functionality to global setting | ||
| Update toggle calls for speed and precision |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,28 +15,45 @@ | |
| } | ||
| var Overlay = (function () { | ||
| function Overlay() { | ||
| this.width = g.getWidth() - 2 * 2; | ||
| this.height = g.getHeight() - 24 - 10; | ||
| this.g2 = Graphics.createArrayBuffer(this.width, this.height, 4, { msb: true }); | ||
| this.g2.transparent = 13; | ||
| this.renderG2(); | ||
| } | ||
| Overlay.prototype.setBottom = function (bottom) { | ||
| var g2 = this.g2; | ||
| var y = bottom - this.height; | ||
| Bangle.setLCDOverlay(g2, 2, y - 10); | ||
| }; | ||
| Overlay.prototype.hide = function () { | ||
| Bangle.setLCDOverlay(); | ||
| }; | ||
| Overlay.prototype.renderG2 = function () { | ||
| var border = 3; | ||
| this.g2 | ||
| .reset() | ||
| .setColor(13) | ||
| .fillRect(0, 0, this.width, this.height) // Background (Transparent) | ||
|
|
||
| .setColor(colour.on.bg) | ||
| .fillRect({ | ||
| x: 0, | ||
| y: 0, | ||
| w: this.width, | ||
| h: this.height, | ||
| r: 20 | ||
| }) // The Outer Shape | ||
|
|
||
| .setColor(g.theme.bg) | ||
| .fillRect({ | ||
| x: border, | ||
| y: border, | ||
| w: this.width - (border * 2), | ||
| h: this.height - (border * 2), | ||
| r: 16 | ||
| }); | ||
| }; | ||
| return Overlay; | ||
| }()); | ||
| var colour = { | ||
|
|
@@ -46,7 +63,7 @@ | |
| }, | ||
| off: { | ||
| fg: "#000", | ||
| bg: g.theme.dark?"#fff":"#bbb", | ||
| }, | ||
| }; | ||
| var Controls = (function () { | ||
|
|
@@ -59,15 +76,24 @@ | |
| { x: width / 4 - 10, y: centreY - circleGapY }, | ||
| { x: width / 2, y: centreY - circleGapY }, | ||
| { x: width * 3 / 4 + 10, y: centreY - circleGapY }, | ||
| { x: width / 4 - 10, y: centreY + circleGapY }, | ||
| { x: width / 2, y: centreY + circleGapY }, | ||
| { x: width * 3 / 4 + 10, y: centreY + circleGapY }, | ||
| ].map(function (xy, i) { | ||
| var ctrl = xy; | ||
| var from = controls[i]; | ||
|
|
||
| // Wrap into a cb function | ||
| ctrl.cb = function(tap) { | ||
| if (tap) return from.toggle(); | ||
| return from.get(); | ||
| }; | ||
|
|
||
| ctrl.text = from.text; | ||
| ctrl.img = from.img; | ||
| Object.assign(ctrl, ctrl.cb(false) ? colour.on : colour.off); | ||
|
|
||
| return ctrl; | ||
| }); | ||
| } | ||
| Controls.prototype.draw = function (g, single) { | ||
|
|
@@ -80,7 +106,7 @@ | |
| .setColor(ctrl.bg) | ||
| .fillCircle(ctrl.x, ctrl.y, 23) | ||
| .setColor(ctrl.fg) | ||
| .drawImage(ctrl.img,ctrl.x-12,ctrl.y-12) | ||
RKBoss6 marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. The image is drawn at a fixed offset (ctrl.x-12, ctrl.y-12) which assumes all images are 24x24 pixels. If images of different sizes are used in the future, this could cause misalignment. Consider adding image width/height properties or centering logic.
RKBoss6 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| }; | ||
| Controls.prototype.hitTest = function (x, y) { | ||
|
|
@@ -109,79 +135,71 @@ | |
| var initUI = function () { | ||
| if (ui) | ||
| return; | ||
| // ... inside initUI ... | ||
RKBoss6 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| var controls = [ | ||
| { | ||
| text: "BLE", | ||
| img:atob("GBiBAAAAAAAYAAAcAAAfAAAbgAAZ4AYYYAcY4AOZwAH/AAB+AAA8AAA8AAB+AAD/AAOZwAcY4A4YYAQZ4AAbgAAfAAAcAAAYAAAAAA=="), | ||
| get: () => { | ||
| const status = NRF.getSecurityStatus(); | ||
| return status.advertising || status.connected; | ||
| }, | ||
| toggle: () => { | ||
| if (NRF.getSecurityStatus().advertising||NRF.getSecurityStatus().connected) NRF.sleep(); else NRF.wake(); | ||
| } | ||
| }, | ||
| { | ||
| text: "DnD", | ||
| img:atob("GBiBAAAAAAAAAAA8AAAYAAAYAAD/AAH/gAH/gAP/wAP/wAP/wAP/wAP/wAP/wAP/wAf/4Af/4A//8B//+A//8AAAAAA8AAAAAAAAAA=="), | ||
| get: () => { | ||
| return require("Storage").readJSON("setting.json", 1).quiet === 1; | ||
| }, | ||
| toggle: () => { | ||
| let s = require("Storage").readJSON("setting.json", 1); | ||
| s.quiet = s.quiet ? 0 : 1; | ||
RKBoss6 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| require("Storage").writeJSON("setting.json", s); | ||
RKBoss6 marked this conversation as resolved.
Show resolved
Hide resolved
Collaborator
There was a problem hiding this comment. I think we should make this optional, I want the quiet mode to be temporary and also avoid the flash writes on each tap
Contributor
Author
There was a problem hiding this comment. My only gripe with that is when I first used the control panel, bluetooth wouldn't connect, and it took me a few days before I figured out that it was locally turning bluetooth off, and just wouldn't display that. I would prefer to have it global so all apps are affected but haptics still work, but possibly avoiding flash writes if we can.
Collaborator
There was a problem hiding this comment. I think there's a mix-up here? This is quiet mode, not bluetooth?
Contributor
Author
There was a problem hiding this comment. Sorry, I was using it as an example. The point was I didn't want any local change to quiet mode when there's already a global implementation. Could we have a global variable used in messages instead that is saved on e. Kill? I think that would be a better solution for system-level quiet mode, as it avoids writes unnecessarily until E.kill is triggered, and can just be appended in bootloader for simplicity.
Collaborator
There was a problem hiding this comment. Ah I see - my use case is different - I want a global change to just buzz but I don't want quiet mode installed or to have it as a hard dependency of the app. Perhaps have the code check if quiet mode scheduler or similar is installed and enable the functionality in that case? And yes, agreed - saving quiet mode setting in the
Contributor
Author
There was a problem hiding this comment. Sounds good! For the e.kill handler, we'll need a global variable to act in the file's place, which we have to create. Should we do that, and then update all the apps that use quiet mode? Alternatively, we could have a more system level one, and create a
Member
There was a problem hiding this comment. I really don't like the idea of starting to mess with other apps and adding global variables just to deal with this case. So the reason you want to save is because the messages lib looks at It looks like you and @bobrippling want different things, and as he says maybe you need it to be an option. Either:
I think probably the best bet it to switch to just changing Interestingly internally Bangle.js actually reads the I guess
Contributor
Author
There was a problem hiding this comment. That sounds good! As for this however, I can make it a setting and use that to decide whether we want buzzes at all, or only quiet mode for messages, etc. It might take some time though, as I'm a bit busy now, but I'll try to get it done soon. |
||
| //if qm widget is present, update that | ||
RKBoss6 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if (typeof WIDGETS === "object" && "qmsched" in WIDGETS) WIDGETS["qmsched"].draw(); | ||
| if (global.setAppQuietMode) setAppQuietMode(s.quiet); | ||
| } | ||
| }, | ||
| { | ||
| text: "HRM", | ||
| img:atob("GBiBAAAAAA+B8B/D+D/n/H///v/////////P///P///H/3+WQAC2Xj82HB86uA/48Af54AP9wAH9gAD/AAB+AAA8AAAYAAAAAAAAAA=="), | ||
| get: () => Bangle.isHRMOn(), | ||
|
Collaborator
There was a problem hiding this comment. This will mislead users - tapping the button will have no visible effect if another app/widget has enabled it (it'll remain on)
Contributor
Author
There was a problem hiding this comment. Hmm, how do you propose we go about fixing it? It has tripped me up when I used it, so is there a way to distinguish whether we triggered it, or if it's on in general? I suppose if we really want the 'control center feel' that we get on phones, it might be best to have it as it is now, as it just shows the state of the HRM and allows you to toggle...
Collaborator
There was a problem hiding this comment. There is - we can either have a boolean to keep track if we triggered it, or we delve into Bangle internals to find out, which is what the code does atm
Contributor
Author
There was a problem hiding this comment. Gotya, let me look and see how it's done now to see if we can add that functionality in!
Collaborator
There was a problem hiding this comment. Sounds good - the code had the checks in, you could pinch that: BangleApps/apps/ctrlpad/main.ts Lines 220 to 221 in 1782ced |
||
| toggle: () => { | ||
| Bangle.setHRMPower(!Bangle.isHRMOn(), "widhid"); | ||
| } | ||
| }, | ||
| { | ||
| text: "clock", | ||
| img:atob("GBiBAAB+AAP/wAeB4A4AcBgAGDAADHAYDmAYBmAYBsAYA8AYA8AYA8AcA8AOA8AHA2ADBmAABnAADjAADBgAGA4AcAeB4AP/wAB+AA=="), | ||
| get: () => false, // Always looks "off" until pressed | ||
RKBoss6 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| toggle: () => { | ||
| Bangle.showClock(); | ||
| return "close"; | ||
| } | ||
| }, | ||
| { | ||
| text: "launch", | ||
| img:atob("GBiBAAAAAAAAAAAAAA/AwB/gwBhgwBhn+Bhn+BhgwB/gwA/AwAAAAAAAAA/D8B/n+BhmGBhmGBhmGBhmGB/n+A/D8AAAAAAAAAAAAA=="), | ||
| get: () => false, | ||
| toggle: () => { | ||
| Bangle.showLauncher(); | ||
| return "close"; | ||
| } | ||
| }, | ||
| { | ||
| text: "settings", | ||
| img:atob("GBiBAAA8AAB+AAR+IA7/cB//+D///B//+A/D8B8A+H8A/v4Af/4Af/4Af/4Af38A/h8A+A/D8B//+D///B//+A7/cAR+IAB+AAA8AA=="), | ||
| get: () => false, | ||
| toggle: () => { | ||
| Bangle.load("setting.app.js"); | ||
| return "close"; | ||
| } | ||
| } | ||
| ]; | ||
|
|
||
| var overlay = new Overlay(); | ||
| ui = { | ||
| overlay: overlay, | ||
|
|
@@ -218,7 +236,7 @@ | |
| break; | ||
| case 0: | ||
| if (e.b && !touchDown) { | ||
| if (e.y <= 10) { | ||
| state = 1; | ||
| startY = e.y; | ||
| (_a = E.stopEventPropagation) === null || _a === void 0 ? void 0 : _a.call(E); | ||
|
|
@@ -289,6 +307,30 @@ | |
| if (e.b) | ||
| touchDown = true; | ||
| }); | ||
| var onCtrlTap = function(ctrl) { | ||
| Bangle.buzz(20); | ||
|
|
||
| var result = ctrl.cb(true); | ||
| if (result === "close") { | ||
| terminateUI(); | ||
| return; | ||
| } | ||
|
Comment on lines
+314
to
+317
Collaborator
There was a problem hiding this comment. I think it's ok to have the callback terminate the UI itself, rather than spread the responsibility and allocate strings in the code
Contributor
Author
There was a problem hiding this comment. This was due to ui being undefined when it's called, because it's defined later in scope, so this just passes it down to a place where it can actually terminate the UI first.
Collaborator
There was a problem hiding this comment. Can you show me the code where that's the problem and the error output? These functions are all executed after they're all defined, so I don't see how it can be a scoping problem. For it being a problem that var result = ctrl.cb(...); // if ui was `undefined` here then
if(result == "close"){
terminateUI(); // there's no code between there and here which will make it defined
return;
} |
||
| ui.ctrls.controls.forEach(function(c) { | ||
| var isActive = c.cb(false); | ||
| Object.assign(c, isActive ? colour.on : colour.off); | ||
| }); | ||
|
|
||
| // Clear and Redraw the buffer | ||
| ui.overlay.renderG2(); | ||
| ui.ctrls.draw(ui.overlay.g2); | ||
RKBoss6 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| // Force an update through the overlay | ||
| var y = g.getHeight() - ui.overlay.height; | ||
| Bangle.setLCDOverlay(ui.overlay.g2, 2, y - 10); | ||
| Bangle.buzz(10); | ||
|
|
||
| }; | ||
|
|
||
| var onTouch = (function (_btn, xy) { | ||
| var _a; | ||
| if (!ui || !xy) | ||
|
|
@@ -301,14 +343,7 @@ | |
| (_a = E.stopEventPropagation) === null || _a === void 0 ? void 0 : _a.call(E); | ||
| } | ||
| }); | ||
|
|
||
| Bangle.prependListener("drag", onDrag); | ||
| Bangle.on("lock", terminateUI); | ||
| })(); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the
13all about here?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah- it was just a number in the palette that's used as a transparent background color for the corners. It's the best solution I was able to come up with, but if you have another way, I would prefer having a more elegant solution over this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotya - I don't know of any way to set transparent otherwise. Do you have the source palette for us to see how likely it is that this will change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what you mean by source palette :( If it helps, in the screenshot before I edited the layers on, the transparent color was shown as an orange..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I mean just where the 13 comes from, like where you found it - the image bytes themselves?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, like I said before, it's just a random number I chose, with no significance at all. That probably isn't the best way to go about something like that, but I didn't find another way :(
Do we need to change it/actually source the value from somewhere? It has worked for all I've been using it, you can upload the boot.js to your watch as well if you need to test it.
(I could still be misunderstanding what you're trying to ask, I'm sorry for the confusion)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see! I think 13 is ok then, let's make it clear in a comment that it's a somewhat arbitrary variable chosen out of nowhere in particular :)