fix(card-template-editor): prevent button flickering with 4+ card types

[xenonnn4w]

Purpose / Description

Template editor buttons (BottomNavigationView with Front/Back/Styling options) flicker and sometimes disappear when switching between card types in note types with 4+ cards.

Fixes

• Fixes Template editor buttons disappear when having more than 3 card types #18555

Approach

Set ViewPager2.offscreenPageLimit = 3 to prevent fragment destruction during tab transitions. The flicker occurs because both old and new fragments' MenuProviders are active simultaneously during ViewPager2 transitions, and lifecycle-based fixes don't work since both fragments remain in RESUMED state. Bounded at 3 to balance flicker prevention with memory usage, keeping up to 7 fragments alive..

How Has This Been Tested?

• Created a note type with 5+ card templates
• Opened the Card Template Editor
• Rapidly switched between distant tabs

Tested on -

Learning (optional, can help others)

ViewPager2 uses RecyclerView internally with a default caching strategy that only keeps adjacent pages
https://developer.android.com/reference/androidx/viewpager2/widget/ViewPager2#setOffscreenPageLimit(int)

Checklist

Please, go through these checks before submitting the PR.

• You have a descriptive commit message with a short title (first line, max 50 chars).
• You have commented your code, particularly in hard-to-understand areas
• You have performed a self-review of your own code
• UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
• UI Changes: You have tested your change using the Google Accessibility Scanner

[mikehardy]

My immediate thought when I read this:

By setting offscreenPageLimit to the template count, all fragments remain in memory and no destruction/recreation occurs.

...was "okay, so if I have very large templates (and some people have entire javascript bundles in there practically, along with like tailwind CSS...) and I have 100 of them (or, get ridiculous, 1000...) am I now open to an OutOfMemory error?

Stated more generically: we are dealing with user-provided boundless input (card templates), and using a bounded system (device memory)

Indeed, the API docs are prescriptive on this point:

You should keep this limit low, especially if your pages have complex layouts

Is there some way to put a reasonable cap on the number of templates we keep in memory, just in case? Say, 10? That will still leave us with flicker though - it seems that there will be an unavoidable flicker with our current code if we accept the limits of device memory and have a limit on in-memory fragments.

I think the correct solution is to figure out where in the lifecycle things have gone wrong leading to a temporarily incorrect state + flicker, and to use some of the provided callback APIs to register interest in lifecycle stages where we can update the UI as needed at the correct time so we have correct UI state and no flicker

ViewPager2 has some scrolling callbacks, the adapter inside of it has a few callbacks

In that sense, the current state where only 4 templates causes a flicker is in fact a good test case for the real fix

Then perhaps it is interesting to keep more templates off screen but that is just a performance / smoothness fix - a separate thing - vs a UI correctness / anti-flicker fix

[xenonnn4w]

You should keep this limit low, especially if your pages have complex layouts

Thank you Mike for that detailed feedback, i'also had the same in my mind but this was initial plan, got busy so i pushed it right there. Now I investigated this thoroughly-

logcat

21:01:25.880 - Fragment[0] setupMenu - adding MenuProvider
21:01:25.894 - Fragment[4] onCreateMenu - inflating menu, WRONG fragment!
21:01:26.247 - Fragment[4] onDestroyView
21:01:26.261 - Fragment[0] onCreateMenu - done 

The old fragment's MenuProvider fires onCreateMenu 400ms before it's destroyed, causing the flicker. Both fragments are RESUMED simultaneously during the transition.

Lifecycle approaches i tried ():

• Activity-level onCreateOptionsMenu/onPrepareOptionsMenu with delegation - tests fail
• Activity-level addMenuProvider with currentFragment delegation - fragment is null when menu created
• ViewPager2 onPageSelected + invalidateOptionsMenu() - flicker persists (doesn't prevent old fragment's callback)
• ViewPager2 onPageScrollStateChanged state tracking - flicker persists
• Fragment's onCreateMenu checking if it's the current fragment - tests fail

umm i feel offscreenPageLimit is the only solution:
The framework calls onCreateMenu on ALL resumed fragments. Since ViewPager2 keeps both old and new fragments resumed during transitions, we can't intercept this. The only way to prevent the wrong fragment's menu from appearing is to keep it alive (not destroyed), so its MenuProvider stays registered but stable. As per the feedback, to avoid OOM error the bound will be 3 i.e.
This keeps up to 7 fragments in memory (current ± 3)

[mikehardy]

The old fragment's MenuProvider fires onCreateMenu 400ms before it's destroyed,

That's most strange - especially since you have a log in there where you know it is the "wrong" fragment 🤔 how did you know it was the wrong fragment? What's the code path that does onCreateMenu ?

Seems like a bug, basically - like that onCreateMenu call should either not happen (by fixing whatever bug in our code causes it to happen or subclassing framework code and no-oping it somehow if possible, or ... some other idea), or if you were able to know it was the wrong fragment (somehow?) could maybe stop it from creating it's menu as well

If there is really no way to do a clean solution where the menu is just working correctly by showing the correct fragment, then having it work well for a "reasonable" number of fragments at least does seem fine. In that context, 7 is probably pretty conservative, could likely handle 10 or 20 without worry if we're being honest - just not unbounded.

[xenonnn4w]

That's most strange - especially since you have a log in there where you know it is the "wrong" fragment 🤔 how did you know it was the wrong fragment?

Added debug logging that includes the fragment's ordinal (arguments?.getInt(CARD_INDEX)). The "wrong" fragment is whichever one doesn't match viewPager.currentItem at that moment.

What's the code path that does onCreateMenu ?

Each CardTemplateFragment registers its own MenuProvider in setupMenu():

requireActivity().addMenuProvider(menuProvider, viewLifecycleOwner, Lifecycle.State.RESUMED)

When the menu is invalidated (via invalidateOptionsMenu() or lifecycle changes), the MenuHost calls onCreateMenu on all registered MenuProviders whose lifecycle owner is in RESUMED state. During ViewPager2 transitions, both old and new fragments are RESUMED simultaneously, so both get the callback.

Could we check if it's the wrong fragment and skip menu creation?

Tried this! Added a check in onCreateMenu:

val currentPosition = (requireActivity() as CardTemplateEditor).viewPager.currentItem
if (arguments?.getInt(CARD_INDEX) != currentPosition) return 

But Robolectric tests failed - shadowOf(activity).clickMenuItem() returns false because the menu isn't properly set up when we skip inflation. The framework expects onCreateMenu to actually inflate the menu.

Could we subclass/no-op the framework call?

The MenuHost.addMenuProvider() API doesn't provide hooks to intercept which providers get called. We need to either:

• Not use per-fragment MenuProviders (move to activity-level); but then we lose the automatic lifecycle cleanup
• Manually remove/re-add MenuProviders during transitions; but the timing is tricky and ViewPager2 doesn't expose reliable "transition complete" callbacks

Conclude:
Happy to bump it to offscreenPageLimit = 5 (keeping 11 fragments)(covers virtually all real-world cases imo)

[mikehardy]

No fault at all of your own, but this is so lame 😆
I relent, "works for me"
Leaving to second reviewer a taste check for:

"how many should we keep to cover a few standard deviations of likely template counts?"

7 total seems fine? 10 or 15 would probably cover 99.9999% of cases. Worth increasing slightly? 🤷

[BrayanDSO]

7 is generally considered a lucky number