Skip to content

feat(review-reminders): prep for edge to edge - #21588

Merged
david-allison merged 6 commits into
ankidroid:mainfrom
david-allison:edge-to-edge-schedule-reminders-infra
Aug 26, 2026
Merged

feat(review-reminders): prep for edge to edge#21588
david-allison merged 6 commits into
ankidroid:mainfrom
david-allison:edge-to-edge-schedule-reminders-infra

Conversation

@david-allison

@david-allison david-allison commented Aug 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Note

Assisted-by: Claude Fable 5

Issues

Approach

Various refactors, then add tests: both insets and screenshots

How Has This Been Tested?

This is effectively test-only. Screenshot tests will expose issues

Checklist

  • 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

@david-allison
refactor(study-options): consume insets
f360753 
The study options containers are padded with the system bar insets,
consuming them means they will not be applied twice.

Non-functional prep for removing FragmentHost.supportsEdgeToEdge.

Part of 17334
Part of 21519

Assisted-by: Claude Fable 5
@david-allison
refactor: add View.doOnApplyWindowInsets
707c3f3 
Helper for Insets which provides the initial insets to the lambda

Issue 17334

Assisted-by: Claude Fable 5
@david-allison
refactor: dp.plus: 1.dp + 1.dp
4760ab4 
Issue 17334

Assisted-by: Claude Fable 5
@david-allison
fix(tests): clear reviewReminderDeserializationErrors
9a73526 
Issue 17334, 21519

Assisted-by: Claude Fable 5
@david-allison
test(review-reminders): inset handling
826db77 
Issue 17334, 21519 - edge to edge

Assisted-by: Claude Fable 5
@david-allison david-allison added the Review High Priority Request for high priority review label Aug 24, 2026
@david-allison david-allison mentioned this pull request Aug 24, 2026
Open
5 tasks
@github-actions

github-actions Bot commented Aug 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Snapshot diff report vs main. Open screenshot-diff for diffs.

  • ReviewRemindersScreenshotTest: 10 changes
All 10 changed screenshots

ReviewRemindersScreenshotTest

  • settingsHostTablet_scheduleReminders_compare.png
  • settingsHost_landscapeCutout_collapsed_compare.png
  • settingsHost_landscapeCutout_compare.png
  • settingsHost_landscapeCutout_rtl_compare.png
  • settingsHost_scheduleReminders_compare.png
  • settingsHost_scheduleReminders_scrolled_compare.png
  • standaloneActivityHost_scheduleReminders_compare.png
  • standaloneActivityHost_systemBars_compare.png
  • standaloneActivityHost_systemBars_scrolledToEnd_compare.png
  • standaloneActivityHost_troubleshooting_systemBars_compare.png

ericli3690
ericli3690 approved these changes Aug 25, 2026
View reviewed changes

@ericli3690 ericli3690 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! Thanks for all your work here, I know first-hand how wacky it is to support all of ScheduleReminders' FragmentHosts. Feel free to ping me on the follow-up PR once it's undrafted and I'll take a look as soon as possible.

Just a note on the screenshot tests, I'm not sure if this is intentional but some of the screenshots look a little wonky, maybe it's just Robolectric being Robolectric. For example (see left edge, top edge, bottom edge):

Image

Comment thread AnkiDroid/src/main/java/com/ichi2/utils/ViewUtils.kt
Comment thread AnkiDroid/src/test/java/com/ichi2/anki/reviewreminders/ReviewRemindersInsetsTest.kt
Comment thread AnkiDroid/src/test/java/com/ichi2/anki/reviewreminders/ReviewRemindersScreenshotTest.kt Outdated
* top notch is rotated to landscape
*/
@SuppressLint("RtlHardcoded") // insets and cutouts are physical: not layout-direction relative
private fun Activity.simulateSystemBars(cutoutLeft: Dp = 0.dp) {

@ericli3690 ericli3690 Aug 25, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: This function seems to be somewhat general-purpose; consider moving it to a more centralized location than the bottom of ReviewRemindersScreenshotTest, especially as more edge-to-edge test code is written? I might be misreading this code though

@david-allison david-allison Aug 26, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Comment thread AnkiDroid/src/test/java/com/ichi2/anki/reviewreminders/ReviewRemindersScreenshotTest.kt
}

/** Inserts [count] reminders so the list has content to render behind the simulated bars */
private fun insertReminders(count: Int) {

@ericli3690 ericli3690 Aug 25, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice; FYI, I plan on adding a few more screenshot tests for cases like deleted review reminders, toggled-off review reminders, no review reminders, etc. shortly

@ericli3690 ericli3690 added Needs Second Approval Has one approval, one more approval to merge and removed Needs Review labels Aug 25, 2026
@david-allison

david-allison commented Aug 25, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

Thanks!!

The issues in the screenshots are intentional, to generate the "before" screenshots.

They are too be fixed in the drafted PR

@david-allison
test(review-reminders): screenshots for insets
a2537e2 
Baseline captures across the FragmentHosts, with simulated system
bars: the edge-to-edge fixes then show up as image diffs.

Adds an id to the troubleshooting scroll view so the test can
scroll it.

Issue 17334, 21519

Assisted-by: Claude Fable 5
@david-allison
david-allison force-pushed the edge-to-edge-schedule-reminders-infra branch from bc5b320 to a2537e2 Compare August 26, 2026 19:35
@david-allison

david-allison commented Aug 26, 2026

Copy link
Copy Markdown
Member Author

I'm going to merge as this is test-only to move the following along:

@david-allison
david-allison added this pull request to the merge queue Aug 26, 2026
Merged via the queue into ankidroid:main with commit 1f16cdd Aug 26, 2026
22 checks passed
@github-actions github-actions Bot removed Review High Priority Request for high priority review Needs Second Approval Has one approval, one more approval to merge labels Aug 26, 2026
@github-actions github-actions Bot added this to the 2.25 release milestone Aug 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ericli3690 ericli3690 ericli3690 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

2.25 release

Development

Successfully merging this pull request may close these issues.

Edge to edge: clean up ScheduleRemindersFragment

2 participants

@david-allison @ericli3690