Skip to content

Edge to edge fix#3616

Merged
domonkosadam merged 4 commits intomasterfrom
Edge-to-edge-fix
Apr 22, 2026
Merged

Edge to edge fix#3616
domonkosadam merged 4 commits intomasterfrom
Edge-to-edge-fix

Conversation

@domonkosadam
Copy link
Copy Markdown
Contributor

No description provided.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 1, 2026

🧪 Unit Test Results


📊 Summary

  • Total Tests: 0
  • Failed: 0
  • Skipped: 0
  • Status: ⚠️ No test results found

Last updated: Wed, 22 Apr 2026 09:12:49 GMT

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.

Review Summary

This PR swaps the IME (keyboard) inset handling in EdgeToEdgeScaffold from a contentWindowInsets-based approach to a Modifier.imePadding() approach. The intent is clear and the change is small, but it carries a meaningful behavioral difference worth documenting.


Issues

  • Unused import (EdgeToEdgeScaffold.kt, line 19) — import androidx.compose.foundation.layout.WindowInsets is no longer referenced after removing the .union(WindowInsets.ime) call. The IDE/lint will flag this as an unused import.

  • Behavioral change: imePadding() on the Scaffold modifier affects the entire Scaffold, not just the content area (EdgeToEdgeScaffold.kt, line 54)

    The two approaches behave differently:

    Old (contentWindowInsets.union(WindowInsets.ime)) New (modifier.imePadding())
    Content bottom padding when keyboard open Grows by IME height Unchanged (Scaffold already shrunk)
    bottomBar position when keyboard open Fixed — can be hidden behind keyboard Floats above keyboard
    Top bar when keyboard open Unaffected Unaffected (correct)
    IME insets consumed? No (only declared in contentWindowInsets) Yes — inner imePadding() calls become no-ops

    If the motivation is to keep a non-empty bottomBar visible above the keyboard, the new approach achieves that correctly. But callers that expect paddingValues.calculateBottomPadding() (passed in the content lambda) to include IME height will need to be aware it no longer does — the Scaffold container itself has already shrunk.

    Also, since imePadding() consumes the IME insets, any composable inside content that also applies imePadding() will silently receive zero insets. This is correct (prevents double-padding) but can be surprising. A code comment noting this intent would help future maintainers.


Positive Notes

  • The change is minimal and easy to reason about.
  • Removing the union(WindowInsets.ime) from contentWindowInsets is consistent with the new modifier-based approach — no double-handling of IME insets within the Scaffold's content slot.
  • Clean import cleanup (removing ime and union).

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 1, 2026

📊 Code Coverage Report

✅ Student

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

✅ Teacher

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

✅ Pandautils

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

📈 Overall Average

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

@domonkosadam domonkosadam merged commit ab358a1 into master Apr 22, 2026
25 of 26 checks passed
@domonkosadam domonkosadam deleted the Edge-to-edge-fix branch April 22, 2026 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants