Skip to content

Commit dda47a5

Browse files
Mearmanclaude
andcommitted
applib/menu_layer: keep a mid-pan reload off the finger-owned offset
menu_layer_update_caches finishes by re-selecting the current row with MenuRowAlignNone, and prv_corrected_scroll_align promotes that to MenuRowAlignCenter on center-focused menus. A reload_data arriving mid-pan (timer, inbox, timeline update) therefore snaps the scroll offset to re-centre the selection under a finger that is still down -- and the next pan update computes from the gesture's base offset, jumping the content straight back. The code's own invariant says menu_layer_set_selected_index cannot be used mid-pan for exactly this reason. Keep the promotion, but not while a touch gesture owns the menu: the finger owns the offset until liftoff, when the carousel settles through prv_menu_touch_settle_to_center as before. Callers that genuinely want a re-centre pass MenuRowAlignCenter explicitly and are unaffected, and the button-path reconcile still promotes because no gesture is active when buttons are pressed. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Joseph Mearman <joseph@mearman.co.uk>
1 parent e7fe4f6 commit dda47a5

2 files changed

Lines changed: 43 additions & 3 deletions

File tree

src/fw/applib/ui/menu_layer.c

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1062,10 +1062,16 @@ static void prv_menu_layer_update_selection_highlight(MenuLayer *menu_layer, boo
10621062
}
10631063

10641064
static MenuRowAlign prv_corrected_scroll_align(MenuLayer *menu_layer, MenuRowAlign align) {
1065-
if (menu_layer->center_focused) {
1066-
return MenuRowAlignCenter;
1065+
if (!menu_layer->center_focused) {
1066+
return align;
10671067
}
1068-
return align;
1068+
#ifdef CONFIG_TOUCH
1069+
// While a touch gesture owns this menu, the finger owns the scroll offset (the carousel settles to the centre on liftoff via prv_menu_touch_settle_to_center, never through here). MenuRowAlignNone must keep its "leave the offset where it is" meaning in that window: a reload's update_caches re-selects the current row with None, and promoting that to a re-centre yanks the offset under the finger, which the next pan update jumps straight back from the gesture's base offset.
1070+
if (menu_layer_touch_is_gesture_target(menu_layer)) {
1071+
return align;
1072+
}
1073+
#endif
1074+
return MenuRowAlignCenter;
10691075
}
10701076

10711077
static void prv_menu_layer_update_selection_scroll_position(MenuLayer *menu_layer,

tests/fw/ui/test_menu_layer.c

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1609,6 +1609,40 @@ void test_menu_layer__dispatch_tap_redirect_selects_target(void) {
16091609

16101610
// ---- Pan through the dispatcher scrolls 1:1 and snaps on liftoff, selection frozen ----
16111611

1612+
// A data reload landing mid-pan on a center-focused carousel must not touch the scroll offset: the finger owns it until liftoff. update_caches re-selects the current row with MenuRowAlignNone, which the center-focused promotion turns into a re-centre -- yanking the offset under the finger, which the next pan update immediately jumps back from the gesture's base offset (a visible double-jump, and a desync for the rest of the gesture).
1613+
void test_menu_layer__dispatch_pan_reload_mid_gesture_keeps_offset(void) {
1614+
prv_touch_nav_setup();
1615+
MenuLayer l;
1616+
menu_layer_init(&l, &GRect(0, 0, 200, 300));
1617+
menu_layer_set_center_focused(&l, true);
1618+
prv_set_touch_callbacks(&l);
1619+
menu_layer_reload_data(&l);
1620+
layer_add_child(&s_root_layer, menu_layer_get_layer(&l));
1621+
const int16_t base = scroll_layer_get_content_offset(&l.scroll_layer).y; // row 0 centred
1622+
prv_reset_touch_counters();
1623+
1624+
prv_drive(TouchEvent_Touchdown, 100, 150);
1625+
prv_advance_ms(20);
1626+
prv_drive(TouchEvent_PositionUpdate, 100, 110); // 40px up -> pan Started
1627+
prv_advance_ms(20);
1628+
prv_drive(TouchEvent_PositionUpdate, 100, 80); // Updated -> live scroll
1629+
cl_assert(menu_layer_touch_is_gesture_target(&l));
1630+
const int16_t mid_pan = scroll_layer_get_content_offset(&l.scroll_layer).y;
1631+
cl_assert(mid_pan != base); // the pan actually moved the content
1632+
1633+
// The reload (timer, inbox, timeline update) lands while the finger is still down.
1634+
menu_layer_reload_data(&l);
1635+
cl_assert_equal_i(scroll_layer_get_content_offset(&l.scroll_layer).y, mid_pan);
1636+
1637+
// Liftoff still settles the carousel: the glide is animated, so (as in the direct-handler settle test) assert it was scheduled rather than driven to its target.
1638+
prv_advance_ms(20);
1639+
prv_drive(TouchEvent_Liftoff, 100, 80);
1640+
Animation *settle = property_animation_get_animation(l.scroll_layer.animation);
1641+
cl_assert(settle != NULL);
1642+
cl_assert(animation_is_scheduled(settle));
1643+
menu_layer_deinit(&l);
1644+
}
1645+
16121646
void test_menu_layer__dispatch_pan_scrolls_1to1_and_snaps(void) {
16131647
prv_touch_nav_setup();
16141648
MenuLayer l;

0 commit comments

Comments
 (0)