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.

Sign up for GitHub

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

Jump to bottom

Discussion: Proposals for new ways to interact with setUI #3452

Closed
wants to merge 1 commit into from
Closed

Discussion: Proposals for new ways to interact with setUI #3452

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 24 additions & 10 deletions apps/spotrem/app.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
let gfx = function() {
widgetUtils.hide();
R = Bangle.appRect;
marigin = 8;

Check warning on line 16 in apps/spotrem/app.js

View workflow job for this annotation

GitHub Actions / build 

'marigin' is not defined
// g.drawString(str, x, y, solid)
g.clearRect(R);
g.reset();
Expand All @@ -21,28 +21,28 @@
if (dark) {g.setColor(0x07E0);} else {g.setColor(0x03E0);} // Green on dark theme, DarkGreen on light theme.
g.setFont("4x6:2");
g.setFontAlign(1, 0, 0);
g.drawString("->", R.x2 - marigin, R.y + R.h/2);

Check warning on line 24 in apps/spotrem/app.js

View workflow job for this annotation

GitHub Actions / build 

'marigin' is not defined

g.setFontAlign(-1, 0, 0);
g.drawString("<-", R.x + marigin, R.y + R.h/2);

Check warning on line 27 in apps/spotrem/app.js

View workflow job for this annotation

GitHub Actions / build 

'marigin' is not defined

g.setFontAlign(-1, 0, 1);
g.drawString("<-", R.x + R.w/2, R.y + marigin);

Check warning on line 30 in apps/spotrem/app.js

View workflow job for this annotation

GitHub Actions / build 

'marigin' is not defined

g.setFontAlign(1, 0, 1);
g.drawString("->", R.x + R.w/2, R.y2 - marigin);

Check warning on line 33 in apps/spotrem/app.js

View workflow job for this annotation

GitHub Actions / build 

'marigin' is not defined

g.setFontAlign(0, 0, 0);
g.drawString("Play\nPause", R.x + R.w/2, R.y + R.h/2);

g.setFontAlign(-1, -1, 0);
g.drawString("Menu", R.x + 2*marigin, R.y + 2*marigin);

Check warning on line 39 in apps/spotrem/app.js

View workflow job for this annotation

GitHub Actions / build 

'marigin' is not defined

Check warning on line 39 in apps/spotrem/app.js

View workflow job for this annotation

GitHub Actions / build 

'marigin' is not defined

g.setFontAlign(-1, 1, 0);
g.drawString("Wake", R.x + 2*marigin, R.y + R.h - 2*marigin);

Check warning on line 42 in apps/spotrem/app.js

View workflow job for this annotation

GitHub Actions / build 

'marigin' is not defined

Check warning on line 42 in apps/spotrem/app.js

View workflow job for this annotation

GitHub Actions / build 

'marigin' is not defined

g.setFontAlign(1, -1, 0);
g.drawString("Srch", R.x + R.w - 2*marigin, R.y + 2*marigin);

Check warning on line 45 in apps/spotrem/app.js

View workflow job for this annotation

GitHub Actions / build 

'marigin' is not defined

g.setFontAlign(1, 1, 0);
g.drawString("Saved", R.x + R.w - 2*marigin, R.y + R.h - 2*marigin);
Expand Down Expand Up @@ -98,25 +98,39 @@
}
};

let buttonHandler = (n)=>{load();};

// Navigation input on the main layout
let setUI = function() {
// Bangle.setUI code from rigrig's smessages app for volume control: https://git.tubul.net/rigrig/BangleApps/src/branch/personal/apps/smessages/app.js
Bangle.setUI(
{mode : "updown",
Bangle.setUI({
mode : "updown",
remove : ()=>{
Bangle.removeListener("touch", touchHandler);
Bangle.removeListener("swipe", swipeHandler);
clearWatch(buttonHandler);
widgetUtils.show();
}
widgetUtils.show();
},
/**
* TODO: Proposal: Handlers added at the same level as the `mode : "updown"` entry
* will replace those that come with the mode - if there were any of that type.
* The handlers in the `extra` object below will still be added in addition to these ones.
**/
swipe : swipeHandler,
btn : buttonHandler,
btnEdge : "rising", // TODO: Proposal: a new, optional, way to choose what edge the button(-s) should act on.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@gfwilliams what do you think of the idea to be able to specify the desired edge like this or some other way?

Copy link
Member

Choose a reason for hiding this comment

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

Well, I was going to say this didn't feel great but I couldn't think of a better way...

But then I was thinking: "What if you want to do different things on Rising and falling edges?"

So what if we had btn (as before) but then btnPress/btnRelease callbacks could be specified? That feels like maybe a tidier solution? What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

... the only downside of this is I guess it's not easily backwards compatible - but if we want this stuff to work outside of the mode:custom then maybe that doesn't matter as it wouldn't work now anyway

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@gfwilliams

I think I agree with your callbacks solution being tidier.

But just regarding this:

But then I was thinking: "What if you want to do different things on Rising and falling edges?"

I'm not saying it's the better solution, just another one. If "both" was specified in my example above that could be passed on to the setWatch call inside setUI and then the buttonHandler function would be responsible for handling both edges.

And if you feel callbacks are better that carries much weight for me :)

//touch : touchHandler, // TODO: Proposal: if added on the top level along `mode : "updown"` this replaces the touch handling of the mode.

/**
* TODO: Proposal: a way to add extra handlers in addition to the ones set up
* by e.g. mode "updown".
**/
extra : {
touch : touchHandler,
//drag : dragHandler // TODO: Proposal: a drag handler here would be set up in addition to the one set up by `mode : "updown"`
},
},
ud => {
if (ud) Bangle.musicControl(ud>0 ? "volumedown" : "volumeup");
}
);
Bangle.on("touch", touchHandler);
Bangle.on("swipe", swipeHandler);
let buttonHandler = setWatch(()=>{load();}, BTN, {edge:'falling'});
};

// Get back to the main layout
Expand Down
Loading