E_showPrompt_Q3: allow long press actions

[thyttan]

as proposed at espruino/BangleApps#3915 (comment).

Pending discussion.

[RKBoss6]

If it works, and is solid, add it to showAlert as well for consistency?

[RKBoss6]

Just a question, how does the changes made here not need a cutting edge fw update? I was under the impression that all changes here need it, but as mentioned in BangleApps issue #3915, it won't need that. How is that possible? Is it an interpreter change and when it goes live it automatically adds on bangles, or is something else going on.

Just curious, thanks :)

[thyttan]

So my intention is to not break old behavior on old firmware but the new long press stuff will require new firmware. So after we're done no change would be noticed until updating firmware, at which point you would be able to long press the sched showPrompt buttons to access the different snooze options.

Edit: So I guess I didn't mean backwards compatible really, but just that it shouldn't break on older firmwares.

[RKBoss6]

Ohh ok thanks. So for testing purposes, we should all install the cutting edge firmware then. Tysm!

[gfwilliams]

IMO it's nicer to have:

          if (e.type===2 /*long press*/ && options.buttonsLong && options.buttonsLong[btns[i]])
            resolve(options.buttonsLong[btns[i]]);
          else
            resolve(options.buttons[btns[i]]);

While promises will ditch the second call to resolve, it's clearer when reading the code that we only expect one to be run?

But otherwise this looks good - I'm happy for it to go in, but it would be good if the docs change went in at the same time (and I don't know if we care about Bangle.js 1? maybe at this point we don't worry)

[thyttan]

I agree with adding that else, will do!

I can look at adding to Bangle.js 1 if it makes sense?

[gfwilliams]

Thanks! Are you able to test on Bangle.js 1 at all? I think it might be good just to keep things working the same across all devices. Then you'll still be able to get the snooze options even on that

[thyttan]

Not able to test aside from on B1 emulator no 🫡

[gfwilliams]

Hmm, actually I just looked and it's going to be hard to do, because right now it just uses the updown setUI mode, and everything responds on the rising edge anyway. Let's leave it for Bangle.js 1 then.

[RKBoss6]

Let's add this for showAlert as well to keep things consistent, as it could also benefit from long-press actions...

[gfwilliams]

Let's add this for showAlert as well to keep things consistent, as it could also benefit from long-press actions...

showAlert should be find as it calls into showPrompt anyway. If you supply buttonsLong as an option it should 'just work': https://github.com/espruino/Espruino/blob/master/libs/js/banglejs/E_showAlert.js

[thyttan]

This is ready to go in I think

[RKBoss6]

When testing this using a bootloader script, it only sends answer back as true or false, numbers don't work. Here's what I used to test it:

E.showPrompt({
  buttons: { Ok: 1, No: 2 },
  buttonsLong: { No: 3 },
})
  .then(answer => {
    // answer is 1 or 2 or 3
    print(answer);
    
  })

The answer always comes back as true or false, not distinguishing between long press and no.

Edit: to clarify, I put the changes to showPrompt here inside a bootloader script to test.

[bobrippling]

This is ready to go in I think

Would you mind updating the typescript too? From:

Espruino/libs/banglejs/jswrap_bangle.c

Line 6076 in d9f3494

"showPrompt<T = boolean>(message: string, options?: { title?: string, buttons?: { [key: string]: T }, buttonHeight?: number, image?: string, remove?: () => void }): Promise<T>;",

to:

showPrompt<T = boolean>(
    message: string,
    options?: {
        title?: string,
        buttons?: { [key: string]: T },
        buttonsLong?: { [key: string]: T },
        buttonHeight?: number,
        image?: string,
        remove?: () => void
    }
): Promise<T>;

i.e.

 showPrompt<T = boolean>(
   message: string,
   options?: {
     title?: string,
     buttons?: { [key: string]: T },
+    buttonsLong?: { [key: string]: T },
     buttonHeight?: number,
     image?: string,
     remove?: () => void,
   }
 ): Promise<T>;

[thyttan]

@bobrippling added your suggestions and attributed to you. Thanks!

[RKBoss6]

Alright, I found the bug, it just defaults to true or false if there is no message or title. Adding those makes it work as intended. This is good to go!
Wonderful job @thyttan !

[thyttan]

@RKBoss6 Ok, thanks! Do you mean there's a bug to fix with showPrompt? Please open an issue and I can take a look maybe. But with another PR in that case :)

[bobrippling]

@bobrippling added your suggestions and attributed to you. Thanks!

Thanks!

[RKBoss6]

@thyttan Not necessarily a bug maybe, but if the user doesn't provide a message, it defaults buttons to resolve true or false.
I might open up an issue later, but right now it's not an inconvenience, as prompts should have messages in them by default.

[gfwilliams]

Thanks! Looks good - just merged