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

smpltmr: adapt to new setui #3644

Merged
merged 2 commits into from
Nov 29, 2024
Merged

smpltmr: adapt to new setui #3644

merged 2 commits into from
Nov 29, 2024

Conversation

thyttan
Copy link
Collaborator

@thyttan thyttan commented Nov 5, 2024

btn -> btnRelease as to not fire unintentionally when long pressing the button to reset the watch.

thyttan added 2 commits November 5, 2024 20:35
btn -> btnRelease as to not fire unintentionally when long pressing the
button to reset the watch.
@thyttan thyttan marked this pull request as ready for review November 16, 2024 19:15
@bobrippling
Copy link
Collaborator

Nice idea - is this something we might want to change in other apps too? And if so, would you say we might want a shorter way of doing that?

@thyttan
Copy link
Collaborator Author

thyttan commented Nov 19, 2024

I'd think there are probably some other ones as well yes. Haven't done any greping to search for them though.

Might be a good idea - how would we go about it?

@bobrippling
Copy link
Collaborator

Yeah, it's a little unfortunate that we'd need the version checking code in other files, which is a few lines of code

if (parseFloat(process.env.VERSION.replace("v","")) < 224.164) {
  uiOptions.btn = ...
} else {
  uiOptions.btnRelease = ...
}

I guess we could trim it down to something like this:

uiOptions[parseFloat(process.env.VERSION.replace("v","")) < 224.164 ? "btn" : "btnRelease"] = ...

and going off a search, apply it to:

apps/angles/app.js
apps/bikespeedo/app.js
apps/calendar/calendar.js
apps/calibration/app.js
apps/clockbg/settings.js
apps/drained/app.js
apps/drained/app.ts
apps/espruinoterm/app.js
apps/folderlaunch/app.js
apps/folderlaunch/app.ts
apps/gipy/app.js
apps/gpstrek/app.js
apps/kbedgewrite/lib.js
apps/messagelist/app.js
apps/mitherm/app.js
apps/openstmap/app.js
apps/pomoplus/app.js
apps/rebbleagenda/app.js
apps/scicalc/app.js
apps/smpltmr/app.js
apps/spacew/app.js
apps/spacew/bench.js
apps/stopwatch/stopwatch.app.js
apps/sudoku/app.js
apps/tempmonitor/tempmonitor.app.js
apps/tetris/tetris.app.js
apps/tinyheads/settings.js
apps/torch/app.js
apps/wrkmem/app.js

But perhaps as another PR, wdyt? Reckon we flag it to Gordon as something that we might want to change in setUI instead?

@thyttan
Copy link
Collaborator Author

thyttan commented Nov 28, 2024

I believe probably for many of those you found there it may not be causing troubles. If there's not a change that's persisted after the app exits it shouldn't matter.

But maybe I haven't really thought that through.

@bobrippling
Copy link
Collaborator

Yeah sounds good, we can sort them when they crop up I reckon 👍

@bobrippling
Copy link
Collaborator

And thanks for the PR!

@bobrippling bobrippling merged commit 73c3273 into espruino:master Nov 29, 2024
1 check passed
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.

2 participants