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

Bangle 1: E.showScroller() triangle drawn over top item #2153

Closed
andrewgoz opened this issue Feb 17, 2022 · 11 comments · Fixed by #2159
Closed

Bangle 1: E.showScroller() triangle drawn over top item #2153

andrewgoz opened this issue Feb 17, 2022 · 11 comments · Fixed by #2159

Comments

@andrewgoz
Copy link
Contributor

Using the sample E.showscroller code from the documentation, when run on the Bangle.js 1 emulator the top triangle is drawn over the list items.

When I originally posted about this, Gordon suggested it was intended to work with widgets. It does look better with widgets enabled, but that would mean the triangle overdraws the widgets instead?

@gfwilliams
Copy link
Member

gfwilliams commented Feb 17, 2022

but that would mean the triangle overdraws the widgets instead?

Only if there are widgets right in the middle, which is reasonably rare since they grow out from either side.

If you have suggestions for where to put the up arrow that'd be better I'm open to ideas :)

@andrewgoz
Copy link
Contributor Author

Are they needed? I just compared a similar UI on the Pebble, which has a similar three-button hardware UI to the Bangle1.

The way they handle a scroller is that they always try to keep the currently selected item in the center of the screen, with the other items above and below it. It's only when the selected item approaches either the top or bottom of the list that the drawing logic forces the highlight up or down as required to highlight the correct item, but also not allowing the top item to drop down and reveal empty space (and same for the bottom item - it doesn't shift up to the middle - the highlight moves down to the bottom of the screen).

That way triangles aren't needed at all.

@andrewgoz
Copy link
Contributor Author

andrewgoz commented Feb 20, 2022

Here is a demo you can copy-paste into the IDE. I've re-written the Bangle 1 (non-Q3) scroller to work in a Pebble-like fashion.

Edit: changed to pass selection flag to draw function, and make selection box drawing optional - still backward compatible with existing code

var scroller;

var testScroller = (function(options) {
  /* options = {
    h = height
    c = # of items
    draw = function(idx, rect)
    select = function(idx)
  }*/
  
if (!options) return Bangle.setUI();
var selected = 0;
var w = Bangle.appRect.w;
var h = Bangle.appRect.h;
var X = Bangle.appRect.x;
var Y = Bangle.appRect.y;

function drawScroller(idx) {
  g.reset();
  // prefer drawing the list so that the selected item is in the middle of the screen
  var y=((h-options.h)/2)-selected*options.h;
  var by=y+options.c*options.h;
  if (by<=h) y += (h-by);
  if (y>0) y = 0;
  // draw
  for (var i=0;i<options.c;i++) {
    if ((idx===undefined)||(idx===i)) {
      if ((y>-options.h+1)&&(y<h)) {
        var y1 = Math.max(Y,Y+y);
        var y2 = Math.min(Y+h-1,Y+y+options.h-1);
        g.setColor((i==selected)?g.theme.fgH:g.theme.fg)
         .setBgColor((i==selected)?g.theme.bgH:g.theme.bg)
         .setClipRect(X,y1,X+w-1,y2);
        if (!options.draw(i,{x:X,y:Y+y,w:w,h:options.h},i==selected)) {
          // border for selected
          if (i==selected) {
            g.setColor(g.theme.fgH)
             .drawRect(X,Y+y,w-1,Y+y+options.h-1)
             .drawRect(X+1,Y+y+1,w-2,Y+y+options.h-2);
          }
        }
      }
    }
    y+=options.h;
  }
}
g.reset().clearRect(X,Y,w-1,h-1);
drawScroller();
Bangle.setUI("updown",dir=>{
  if (dir) {
    selected += dir;
    if (selected<0) selected = options.c-1;
    if (selected>=options.c) selected = 0;
    drawScroller();
  } else {
    options.select(selected);
  }
});
return {
  draw : drawScroller,
  drawItem : drawScroller
};
});

function show() {
  scroller = testScroller({
    h: 27,
    c: 20,
    draw: (idx, r) => {
      g.clearRect(r.x, r.y, r.x + r.w - 1, r.y + r.h - 1)
       .setFontAlign(0, 0)
       .setFont("6x8:2")
       .drawString((selected?"*":"")"Item "+idx, r.x + r.w / 2, r.y + r.h / 2);
      return true;
    },
    select: i => print("You selected "+i)
  });
}
Bangle.loadWidgets();
Bangle.drawWidgets();
show();

@gfwilliams
Copy link
Member

Thanks - but I see this:

screenshot

So I'm not sure how it's apparent to the user that there are any items available after 'Item 8'?

IIRC we never originally had up/down arrows for menus, but they were added at the request of users - so I'm not convinced that just removing them is suddenly going to be fine with those people that complained previously.

You could always submit the change as an app and see how folks feel about it though: https://www.espruino.com/Bangle.js+Modification

@gfwilliams
Copy link
Member

Just to add, maybe we can have the best of both worlds:

What if the top item (Item 0) stays as Item 0 until the page scrolls, and then it turns into an up-arrow. Same with the bottom-most item

@andrewgoz
Copy link
Contributor Author

It isn't immediately apparent that there is more than Item 8, that is true, but as soon as the user starts navigating down it will become apparent there are more options, then when they get to the end it is also obvious that they have reached the end.

All I can say is that over the years I've been with the Pebble, I haven't found the lack of indication a problem, nor have I heard of such complaints.

I'll have a fiddle with adding the triangles back...

In the mean time, I've just edited my example code above to allow the draw function to know if it's drawing the selected item, and also to optionally inhibit drawing of the selection rectangle. That's mostly to match the Pebble appearance. The changes don't affect existing code conventions.

@andrewgoz
Copy link
Contributor Author

andrewgoz commented Feb 22, 2022

Triangles, as requested :-) (edit: close drawPoly()s, edit2: add startidx, edit3: startidx->scroll)

var scroller;

var testScroller = (function(options) {
  /* options = {
    h = height
    c = # of items
    draw = function(idx, rect)
    select = function(idx)
  }*/
  
if (!options) return Bangle.setUI();
var selected = 0;
if (options.scroll) selected=options.scroll;
var w = Bangle.appRect.w;
var h = Bangle.appRect.h;
var X = Bangle.appRect.x;
var Y = Bangle.appRect.y;

function drawScroller(idx) {
  g.reset();
  // prefer drawing the list so that the selected item is in the middle of the screen
  var ty=((h-options.h)/2)-selected*options.h;
  var y=ty;
  var by=y+options.c*options.h;
  if (by<=h) y += (h-by);
  if (y>0) y = 0;
  // draw
  for (var i=0;i<options.c;i++) {
    if ((idx===undefined)||(idx===i)) {
      if ((y>-options.h+1)&&(y<h)) {
        var y1 = Math.max(Y,Y+y);
        var y2 = Math.min(Y+h-1,Y+y+options.h-1);
        g.setColor((i==selected)?g.theme.fgH:g.theme.fg)
         .setBgColor((i==selected)?g.theme.bgH:g.theme.bg)
         .setClipRect(X,y1,X+w-1,y2);
        if (!options.draw(i,{x:X,y:Y+y,w:w,h:options.h},i==selected)) {
          // border for selected
          if (i==selected) {
            g.setColor(g.theme.fgH)
             .drawRect(X,Y+y,w-1,Y+y+options.h-1)
             .drawRect(1,Y+y+1,w-2,Y+y+options.h-2);
          }
        }
      }
    }
    y+=options.h;
  }
  // arrows
  g.setClipRect(X,Y,X+w-1,Y+h-1);
  var m=w/2;
  var pt=[X+m,Y,X+m-14,Y+14,X+m+14,Y+14];
  var pb=[X+m,Y+h,X+m-14,Y+h-14,X+m+14,Y+h-14];
  if (ty<0) {
    g.setColor(g.theme.fg)
     .fillPoly(pt)
     .setColor(g.theme.bg)
     .drawPoly(pt,true);
  }
  if (by>h) {
    g.setColor(g.theme.fg)
     .fillPoly(pb)
     .setColor(g.theme.bg)
     .drawPoly(pb,true);
  }
}
g.reset().clearRect(X,Y,w-1,h-1);
drawScroller();
Bangle.setUI("updown",dir=>{
  if (dir) {
    selected += dir;
    if (selected<0) selected = options.c-1;
    if (selected>=options.c) selected = 0;
    drawScroller();
  } else {
    options.select(selected);
  }
});
return {
  scroll : () => selected,
  draw : drawScroller,
  drawItem : drawScroller
};
});

function show() {
  scroller = testScroller({
    h: 27,
    c: 20,
    draw: (idx, r, selected) => {
      g.clearRect(r.x, r.y, r.x + r.w - 1, r.y + r.h - 1)
       .setFontAlign(0, 0)
       .setFont("6x8:2")
       .drawString((selected?"*":"")+"Item "+idx, r.x + r.w / 2, r.y + r.h / 2);
      return true;
    },
    select: i => print("You selected "+i)
  });
}
Bangle.loadWidgets();
Bangle.drawWidgets();
show();

@gfwilliams
Copy link
Member

Thanks! That looks good to me - it'd be nice to set and accept a scroll parameter as the current version does, so menus can choose to redraw at the same point (eg if coming from a submenu).

Otherwise a PR would be great :)

@andrewgoz
Copy link
Contributor Author

I could never work out what scroll was supposed to do. I tried a local copy of the original scroller (with the options.scroll fix), but couldn't see it do anything. If it was meant to set the initial selection, then 'scroll' isn't a great name. I'm at work now so can't do much, but I'll edit my latest code above with an optional 'startidx' parameter.

@gfwilliams
Copy link
Member

If it was meant to set the initial selection, then 'scroll' isn't a great name

The issue is really that we want it to be compatible with code that also runs on Bangle.js 2 where it really is the amount of pixels scrolled - so adding a separately named item isn't helpful. The idea is if some app copies the scroll value and puts it back in when calling E.showScroller again then it ends up with the same item selected. It doesn't really matter what's in that value.

@andrewgoz
Copy link
Contributor Author

Thanks for explaining the scroll parameter. That also explains the use of the scroll function in the returned object in the Q3 version, which I note is missing in this version. I'll do a PR.

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 a pull request may close this issue.

2 participants