Skip to content

refactor: core UI common#10942

Open
wmontwe wants to merge 6 commits into
thunderbird:mainfrom
wmontwe:refactor-core-ui-common
Open

refactor: core UI common#10942
wmontwe wants to merge 6 commits into
thunderbird:mainfrom
wmontwe:refactor-core-ui-common

Conversation

@wmontwe
Copy link
Copy Markdown
Member

@wmontwe wmontwe commented Apr 24, 2026

Part of #10392

This pull request refactors and reorganizes the core UI 'common' utilities, moving shared code out of the app.k9mail namespace into the new net.thunderbird.core.ui.common package.

  • Consolidate shared UI utilities into core/ui/common: move PreviewDevices* annotations and WindowSizeClass (now backed by a new WindowSizeInfo) into the module, and extend it with a CalculateWindowSizeInfo helper.
  • Drop redundant core/ui/compose/common pieces: BaselineModifier, ImageWithBaseline, ImageWithOverlayCoordinate, VisibilityModifiers, IconsWithBaseline, and IconsWithBottomRightOverlay. These switched to plain Icons/vertical alignment.
  • Relocate DelayedCircularProgressIndicator and the hide modifier into the onboarding-permissions module and update feature modules to depend on core:ui:common where needed.
  • Removed testTagAsResourceId as it's semantic changes are redundant and only need to be applied once on the outmost composable. Use testTag instead.

@wmontwe wmontwe requested a review from a team as a code owner April 24, 2026 10:30
@wmontwe wmontwe requested a review from rafaeltonholo April 24, 2026 10:30
@github-actions github-actions Bot added the tb-team Tasks and features handled by project maintainers label Apr 24, 2026
@wmontwe wmontwe added the report: exclude Exclude changes from user-facing reports (internal, minor, or not relevant to users). label Apr 24, 2026
@wmontwe wmontwe force-pushed the refactor-core-ui-common branch from 8acb94b to e225f62 Compare April 24, 2026 17:33
@wmontwe wmontwe removed the report: exclude Exclude changes from user-facing reports (internal, minor, or not relevant to users). label May 4, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 4, 2026

Validation Passed: All report and feature-flag labels are correctly set.

@wmontwe wmontwe added the report: exclude Exclude changes from user-facing reports (internal, minor, or not relevant to users). label May 4, 2026
Copy link
Copy Markdown
Member

@rafaeltonholo rafaeltonholo left a comment

Choose a reason for hiding this comment

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

There is a crash happening now because of the changes in the CalculateWindowSizeInfo.kt. Everything else is good.

return remember(windowInfo) {
WindowSizeInfo(
screenWidthSizeClass = WindowSizeClass.fromWidth(windowInfo.containerSize.width),
screenHeightSizeClass = WindowSizeClass.fromHeight(windowInfo.containerSize.height),
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.

This is causing the app to crash due to an IllegalArgumentException. The previous version used configuration.screenWidthDp and configuration.screenHeightDp, but we are now using the pixel values, which is leading to this issue.

Stacktrace:

FATAL EXCEPTION: main
  Process: net.thunderbird.android.debug, PID: 13363
  java.lang.IllegalArgumentException: Padding must be non-negative
  at androidx.compose.foundation.layout.internal.InlineClassHelperKt.throwIllegalArgumentException(InlineClassHelper.kt:34)
  at androidx.compose.foundation.layout.PaddingValuesImpl.<init>(Padding.kt:479)
  at androidx.compose.foundation.layout.PaddingValuesImpl.<init>(Padding.kt:0)
  at androidx.compose.foundation.layout.PaddingKt.PaddingValues-YgX7TsA(Padding.kt:280)
  at androidx.compose.foundation.layout.PaddingKt.PaddingValues-YgX7TsA$default(Padding.kt:279)
  at net.thunderbird.core.ui.common.padding.CalculateResponsivePaddingKt.calculateResponsiveWidthPadding(CalculateResponsivePadding.kt:19)
  at app.k9mail.core.ui.compose.designsystem.template.ResponsiveContentKt.ExpandedContent$lambda$1$0(ResponsiveContent.kt:107)
  at app.k9mail.core.ui.compose.designsystem.template.ResponsiveContentKt.$r8$lambda$olhqj8zKdH2OFb37DfkmQIzTcjA(ResponsiveContent.kt:0)
  at app.k9mail.core.ui.compose.designsystem.template.ResponsiveContentKt$$ExternalSyntheticLambda4.invoke(D8$$SyntheticClass:0)
  at androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.kt:122)
  at androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.kt:52)
  at androidx.compose.material3.SurfaceKt$Surface$1.invoke(Surface.kt:130)
  at androidx.compose.material3.SurfaceKt$Surface$1.invoke(Surface.kt:110)
  at androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.kt:122)
  at androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.kt:52)
  at androidx.compose.runtime.CompositionLocalKt.CompositionLocalProvider(CompositionLocal.kt:378)
  at androidx.compose.material3.SurfaceKt.Surface-T9BRK9s(Surface.kt:107)
  at app.k9mail.core.ui.compose.designsystem.atom.SurfaceKt.Surface-_UMDTes(Surface.kt:50)
  at app.k9mail.core.ui.compose.designsystem.template.ResponsiveContentKt.ExpandedContent(ResponsiveContent.kt:102)
  at app.k9mail.core.ui.compose.designsystem.template.ResponsiveContentKt.ResponsiveContent(ResponsiveContent.kt:37)
  at app.k9mail.feature.onboarding.welcome.ui.WelcomeContentKt.WelcomeContent$lambda$0(WelcomeContent.kt:50)

Copy link
Copy Markdown
Member Author

@wmontwe wmontwe May 6, 2026

Choose a reason for hiding this comment

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

This code was sitting on my shelf for a while. I think I'll need pull in changes done at a later time, that will fix this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

report: exclude Exclude changes from user-facing reports (internal, minor, or not relevant to users). tb-team Tasks and features handled by project maintainers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants