Closed Bug 1907599 Opened 4 months ago Closed 2 months ago

Nav Bar CFR does not update the `lastCfrShownTimeInMillis` variable in `Settings.kt`

Categories

(Fenix :: Toolbar, defect, P2)

All
Android
defect

Tracking

(firefox131 fixed)

RESOLVED FIXED
131 Branch
Tracking Status
firefox131 --- fixed

People

(Reporter: npoon, Assigned: skhan, Mentored)

References

(Blocks 1 open bug)

Details

(Keywords: good-first-bug, Whiteboard: [avocado sprint])

Attachments

(1 file)

All CFRs should update the lastCfrShownTimeInMillis variable in Settings.kt after being dismissed. As a part of this ticket, add the following line to the onDismiss lambda for the nav bar, which is here:
context.settings().lastCfrShownTimeInMillis = System.currentTimeMillis(). This is similar to what is done here

See Also: → 1907596, 1907571
Severity: -- → S3
Keywords: good-first-bug
Mentor: npoon

Sarah, will our nav bar CFR still show when we want it to if we start setting lastCfrShownTimeInMillis = System.currentTimeMillis()? Do we still need to set shouldShowNavigationBarCFR = false?

Looks like there are many callers setting context.settings().lastCfrShownTimeInMillis = System.currentTimeMillis(). Perhaps more of this CFR logic can be consolidated and encapsulated inside Settings object functions so we only have to fix the CFRs in one place?

Flags: needinfo?(skhan)
Priority: -- → P2

Hey Chris, thanks for the comment. The nav bar CFR will still show when it should without setting lastCfrShownTimeInMillis = System.currentTimeMillis(). However, there are other CFRs (i.e. Add Private Tab to Home, Inactive Tabs, Tab Auto Close Banner, PWA) that will only appear if a CFR has not been shown in at least 3 days. My guess is that the navigation bar is essential enough in which we don't delay showing its CFR but other CFRs can be delayed a bit until it is shown (to avoid overwhelming the user).

Flags: needinfo?(skhan)

I agree here with Nick's above comment. Navigation bar CFR is the first CFR that we show on home screen today so it does not require to set lastCfrShownTimeInMillis.

Assignee: nobody → skhan
Status: NEW → ASSIGNED
Whiteboard: [avocado sprint]
Pushed by sarahkhan1107@hotmail.com: https://hg.mozilla.org/integration/autoland/rev/6ed90ce056be Update lastCfrShownTimeInMillis for Navigation Bar CFR r=android-reviewers,tchoh
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → 131 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: