Skip to content

[MBL-19896][Student] Fix edge-to-edge Snackbar gap and offline indicator positioning#3630

Merged
kristofnemere merged 3 commits intomasterfrom
MBL-19896-fix-edge-to-edge-offline-snackbar
Apr 16, 2026
Merged

[MBL-19896][Student] Fix edge-to-edge Snackbar gap and offline indicator positioning#3630
kristofnemere merged 3 commits intomasterfrom
MBL-19896-fix-edge-to-edge-offline-snackbar

Conversation

@kristofnemere
Copy link
Copy Markdown
Contributor

@kristofnemere kristofnemere commented Apr 7, 2026

Test plan:

  1. Build and run the Student app on Android 15+ (edge-to-edge is enforced)
  2. Go offline and open the All Courses page (Edit Dashboard)
  3. Try to mark/unmark a course as favourite — verify the Snackbar appears just above the offline indicator banner with no extra gap below it
  4. Verify the offline indicator banner is not hidden behind the Android navigation bar
  5. Navigate into a course (CourseBrowserFragment) while offline — verify the offline indicator is still visible above the navigation bar
  6. Rotate to landscape — verify no gap appears below the bottom navigation bar
  7. Rotate back to portrait — verify the bottom navigation bar extends behind the Android navigation bar with no visible shelf/gap

⚠️ Smoke test required: This change touches core window insets handling in NavigationActivity and EditDashboardFragment. A quick smoke test across the main flows (Dashboard, Courses, Inbox, Calendar, Notifications) is strongly recommended to catch any unexpected regressions.

refs: MBL-19896
affects: Student
release note: Fixed a gap appearing between the Snackbar and the Android navigation bar on the All Courses page in offline mode, and fixed the offline indicator being hidden behind the navigation bar on secondary screens.

  • Follow-up e2e test ticket created or not needed
  • Tested in dark mode
  • Tested in light mode
  • Test in landscape mode and/or tablet
  • A11y checked
  • Approve from product

…tor positioning

- Remove duplicate setOnApplyWindowInsetsListener on bottomBarContainer (the second
  listener was silently overwriting the first, killing offline indicator margin logic)
- Use height extension (56dp + navigationBars.bottom) in both portrait and landscape
  instead of bottomMargin, so the bottom bar draws behind the nav bar without a gap
- Always consume navigationBars.bottom in the fullscreen listener since bottomBarContainer
  already handles that space, preventing double-counting in child views
- Add listener on fullScreenCoordinatorLayout to strip bottom insets before the
  CoordinatorLayout stores them, so Snackbars don't add extra bottom margin regardless
  of whether the offline indicator is visible

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 7, 2026

🧪 Unit Test Results

✅ 📱 Student App

  • Tests: 1210 total, 0 failed, 0 skipped
  • Duration: 0.000s
  • Success Rate: 100%

✅ 🌅 Horizon

  • Tests: 790 total, 0 failed, 0 skipped
  • Duration: 32.760s
  • Success Rate: 100%

✅ 📦 Submodules

  • Tests: 3406 total, 0 failed, 0 skipped
  • Duration: 54.205s
  • Success Rate: 100%

📊 Summary

  • Total Tests: 5406
  • Failed: 0
  • Skipped: 0
  • Status: ✅ All tests passed!

Last updated: Wed, 15 Apr 2026 08:45:17 GMT

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 7, 2026

📊 Code Coverage Report

⚠️ Student

  • PR Coverage: 42.61%
  • Master Coverage: 42.64%
  • Delta: -0.03%

✅ Teacher

  • PR Coverage: 25.31%
  • Master Coverage: 25.31%
  • Delta: +0.00%

⚠️ Pandautils

  • PR Coverage: 23.88%
  • Master Coverage: 23.88%
  • Delta: -0.00%

📈 Overall Average

  • PR Coverage: 30.60%
  • Master Coverage: 30.61%
  • Delta: -0.01%

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 7, 2026

Student Install Page

Comment thread apps/student/src/main/java/com/instructure/student/activity/NavigationActivity.kt Outdated
@adamNagy56 adamNagy56 self-requested a review April 9, 2026 07:17
… position

The fullScreenCoordinatorLayout listener was stripping navigationBars.bottom,
but LinearLayout propagates each child's returned insets to the next sibling,
so offlineIndicator and bottomBarContainer also received bottom=0. This caused
bottomBarContainer's listener to set offlineIndicator.bottomMargin=0, leaving
it behind the system navigation bar in screens like CourseBrowserFragment.

Instead, fix the Snackbar margin surgically in EditDashboardFragment by consuming
insets on the Snackbar view itself — the CoordinatorLayout is already positioned
above the bottomBarContainer so no extra navigationBars.bottom margin is needed.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Copy link
Copy Markdown
Contributor

@adamNagy56 adamNagy56 left a comment

Choose a reason for hiding this comment

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

  • When I’m in offline mode and open any search field, a large gap appears above the keyboard. On the Assignment page, this gap remains even after closing the keyboard. (See 1. attached video.)

  • +1 Teacher app: In portrait mode, the bottom buttons completely overlap the toast message text. (See 2. attached video.)

3852.mp4
Screen_Recording_20260409_153601_Canvas.Teacher.mp4

Copy link
Copy Markdown
Contributor

@kdeakinstructure kdeakinstructure left a comment

Choose a reason for hiding this comment

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

  • Manage Offline content screen Sync button overlaps with android bottom navigation bar

  • All Courses page content overlaps with android bottom navigation bar

Copy link
Copy Markdown
Contributor

@kdeakinstructure kdeakinstructure left a comment

Choose a reason for hiding this comment

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

QA 👍

…es Snackbar regression

- Fix keyboard gap with 3-button navigation: siblings of fullScreenCoordinatorLayout
  receive raw system insets independently, so bottomBarContainer always sets
  offlineIndicator.bottomMargin = navBar.bottom when the bottom bar is hidden.
  Mirror that margin in heightBelowContent so the adjusted IME inset correctly
  accounts for the offline indicator's nav-bar clearance space.
- Replace hardcoded 56.toPx with view.minimumHeight derived from XML minHeight
  so the bottom bar content height is not a magic number in code.
- Fix All Courses Snackbar appearing behind nav buttons in Teacher app: move
  the inset-consuming logic into StudentEditDashboardRouter.showSnackbar() via
  the EditDashboardRouter interface default method, keeping the Student-specific
  fix isolated from the shared EditDashboardFragment.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

PR Review — Update Analysis

This PR fixes a cluster of edge-to-edge window-inset bugs in the Student app: a blank gap above the keyboard when the IME opens, incorrect bottom-bar sizing in both portrait and landscape, and a double-counted nav-bar margin on the Edit Dashboard snackbar.

What changed in this update

Area Change
NavigationActivity Adds IME-inset adjustment (adjustedImeBottom) so child screens subtract the height of views below the content area before padding for the keyboard. Also switches bottom-bar sizing from bottomMargin to explicit height, and removes the now-redundant bottomBarContainer insets listener.
activity_navigation.xml Adds android:minHeight="56dp" so minimumHeight is reliable at runtime.
StudentEditDashboardRouter Overrides showSnackbar to block nav-bar insets from propagating to the Snackbar view when edge-to-edge is enforced.
EditDashboardRouter (interface) Promotes snackbar creation to a default interface method so Teacher/Parent apps get sensible baseline behaviour without changes.
EditDashboardFragment Delegates to editDashboardRouter.showSnackbar(); removes inline Snackbar.make.

Overall assessment

The approach is sound. The inset arithmetic is well-commented and handles the known edge cases (offline indicator visible/hidden, bottom bar visible/hidden, landscape vs portrait). Three focused comments are raised below covering a layout-timing assumption, an overly broad inset consumption, and an implicit contract in the interface default.

Comment on lines +488 to +490
val ime = insets.getInsets(WindowInsetsCompat.Type.ime())
val expectedBottomBarHeight = if (bottomBarContainer.isVisible) (bottomBarContainer.minimumHeight + navigationBars.bottom) else 0
val expectedOfflineMargin = if (bottomBarContainer.isVisible) 0 else navigationBars.bottom
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Potential layout-timing issue with view heights

bottomBarDivider.height and offlineIndicator.root.height could both return 0 if the insets listener fires before these views have been measured and laid out.

In practice, IME insets only arrive when the keyboard opens (after a view gets focus), so the layout should be complete by then — but this is fragile. If the offline indicator becomes visible at the same time the keyboard opens (e.g., network lost while typing), its height might not yet reflect the final value.

Consider defensively using minimumHeight (already set in XML for bottomBarContainer) or requesting a re-dispatch after layout:

// Safer alternative for offlineIndicator height:
val offlineHeight = if (offlineIndicator.root.isVisible)
    maxOf(offlineIndicator.root.height, offlineIndicator.root.minimumHeight) + expectedOfflineMargin
else 0

Or at minimum, add a comment noting the assumption that the IME only appears post-layout.

// In Student app the Snackbar anchors to fullScreenCoordinatorLayout, which is already
// positioned above the bottom bar. Consuming the navigation bar insets prevents the
// Snackbar from adding an extra bottom margin that would double-count the nav bar space.
ViewCompat.setOnApplyWindowInsetsListener(snackbar.view) { _, _ -> WindowInsetsCompat.CONSUMED }
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

WindowInsetsCompat.CONSUMED blocks all inset types, not just navigation bars

Returning CONSUMED intercepts every inset type (display cutout, system gesture exclusions, tappable element insets, IME, etc.), not only the navigation bar inset. This is more aggressive than the comment implies and could prevent the Snackbar from correctly handling future inset types added by Material or AndroidX.

A more surgical fix would be to zero out only the navigation bar portion:

ViewCompat.setOnApplyWindowInsetsListener(snackbar.view) { _, insets ->
    val types = WindowInsetsCompat.Type.navigationBars()
    WindowInsetsCompat.Builder(insets)
        .setInsets(types, Insets.NONE)
        .build()
}

This keeps other inset types intact while still preventing the double-counted nav-bar margin.

interface EditDashboardRouter {
fun routeCourse(canvasContext: CanvasContext?)

fun showSnackbar(fragment: Fragment, resId: Int) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Interface default method introduces a Material library dependency into pandautils

The default showSnackbar implementation imports Snackbar directly into the router interface. Since pandautils already uses Material this is not a blocking concern, but it's worth noting that Teacher/Parent apps that implement EditDashboardRouter without overriding this method will silently inherit the Material-anchored behaviour — including any future edge-to-edge quirks — unless they also override it.

Consider documenting this contract, or making the default a no-op and requiring each app to opt in:

fun showSnackbar(fragment: Fragment, resId: Int) { /* apps must override */ }

This would make it explicit that each consuming app is responsible for its own snackbar positioning, which aligns with the pattern used for routeCourse.

Copy link
Copy Markdown
Contributor

@adamNagy56 adamNagy56 left a comment

Choose a reason for hiding this comment

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

QA +1

@kristofnemere kristofnemere merged commit bc8c1e9 into master Apr 16, 2026
32 checks passed
@kristofnemere kristofnemere deleted the MBL-19896-fix-edge-to-edge-offline-snackbar branch April 16, 2026 07:47
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 this pull request may close these issues.

4 participants