podadrem/spotrem: Fix linter warnings

[thyttan]

On edgewrite keyboard the top left corner would be overwritten with a
square in solid background colour, the size and position of the red back
button of the previous menu.

Reproducible with with edgewrite installed and this code:

E.showMenu({
  "": {title: "hi there", back: ()=>{}},
  "this": ()=>{}
});
//E.showMenu();
require("textinput").input({text:"Top left gets cleard by menu back button removal?"});

First brought up on #3632 (comment).

[bobrippling]

LGTM, happy to merge but will give @retcurve a few days to check it over

[retcurve]

I've got an alternative solution that doesn't involve changes to other apps, but it might be worthwhile still making the changes here in case other keyboards have this same problem.

The issue is being caused by setUI - EdgeWrite calls setUI to set btn, touch, and drag handlers, but no back handler. When setUI is called without a back handler it calls WIDGETS.back.remove(false) which removes the existing handler and also clears the back button area. However in EdgeWrite this is happening after the widget bar has been hidden. So by moving the code to hide the widget bar and draw the initial string to after setUI it means things are happening in the correct order and the black box is immediately overwritten with the text. I'll get a PR raised shortly.

[retcurve]

This problem can also be reproduced with anything else that calls setUI without a back handler after widgets have been hidden, for example this will show the black box cutting into the top line of an alert -

E.showMenu({
  "": {title: "hi there", back: ()=>{}},
  "this": ()=>{}
});
require("widget_utils").hide();
E.showAlert('test');

A potential fix here is to call WIDGETS.back.remove(true) before E.showAlert to get rid of the back button without clearing the screen and then the subsequent setUI call won't have a back button to remove.

E.showMenu({
  "": {title: "hi there", back: ()=>{}},
  "this": ()=>{}
});
require("widget_utils").hide();
if (global.WIDGETS && WIDGETS.back) {
  WIDGETS.back.remove(true);
}
E.showAlert('test');

[thyttan]

@gfwilliams what's your take on this conversation?

[gfwilliams]

I'm with @retcurve - the lint fixes are great and should go in, but I think we should be trying to fix the problem at the source.

I guess for me, the safest fix is just to swap around the code in kbedgewrite so we draw the screen after calling Bangle.setUI, then it doesn't matter. I think that's the way around we normally do it (setUI used to clear the screen originally!).

But this also feels like a setUI bug. When we hide widgets, we set the widget.area to "" so we could just check that and not clear the square if it is? It doesn't solve what happens when .swipeOn is used but maybe we ignore that for now. Assuming that's ok I can poke that into the firmware very quickly.

[thyttan]

I'm with @retcurve - the lint fixes are great and should go in, but I think we should be trying to fix the problem at the source.

👍

I guess for me, the safest fix is just to swap around the code in kbedgewrite so we draw the screen after calling Bangle.setUI, then it doesn't matter. I think that's the way around we normally do it (setUI used to clear the screen originally!).

Just merged that PR from @retcurve 🙏👌

But this also feels like a setUI bug. When we hide widgets, we set the widget.area to "" so we could just check that and not clear the square if it is? It doesn't solve what happens when .swipeOn is used but maybe we ignore that for now. Assuming that's ok I can poke that into the firmware very quickly.

Sounds like a good idea to me!

[gfwilliams]

Ok, I pushed changes to setUI to the main repo, so I think we're good now.

Please can you remove your E.showMenu lines here? I'd like to merge this anyway to fix the lint warnings

[thyttan]

👍

I'll fix this PR later tonight

[gfwilliams]

Great - thanks!