Fix Incorrect Language Font Rendering in KMP Web App#3135
Fix Incorrect Language Font Rendering in KMP Web App#3135Aryan-Baglane wants to merge 4 commits intoopenMF:developmentfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughMifosMobileTheme now computes a FontFamily via Changes
Sequence Diagram(s)sequenceDiagram
participant App as "App"
participant Theme as "MifosMobileTheme"
participant Preloader as "FontFallbackPreloader"
participant Provider as "KptThemeProviderImpl"
participant Content as "Compose Content"
App->>Theme: call MifosMobileTheme(content)
Theme->>Theme: fontFamily() -> FontFamily(NotoSans + scripts)
Theme->>Preloader: invoke FontFallbackPreloader()
Preloader->>Preloader: render invisible Texts to preload fonts
Theme->>Provider: create KptThemeProviderImpl(theme)
Theme->>Content: wrap in CompositionLocalProvider(LocalTextStyle=TextStyle(fontFamily))
Provider-->>Content: provide theme tokens/styles
Content->>Content: render UI with preloaded fonts
(Note: rectangles use default styling; main components and sequential flow are shown.) Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
core/designsystem/src/commonMain/kotlin/org/mifos/mobile/core/designsystem/theme/Type.kt (2)
39-47: OnlyNormalfaces are registered; verify weight fidelity.Lines 39–47 provide only
FontWeight.Normalfiles, while app typography uses Medium/SemiBold/etc. This can flatten emphasis via synthetic weights. If available, add real weight variants (at least for the base Latin family) and verify visual parity for headings/buttons.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/designsystem/src/commonMain/kotlin/org/mifos/mobile/core/designsystem/theme/Type.kt` around lines 39 - 47, The font registration in Type.kt currently only adds Regular files (using Font(..., FontWeight.Normal) for Res.font.NotoSans_*), which forces synthetic weights; update the Font declarations to register real weight variants for the base Latin family (e.g., Res.font.NotoSans_Medium, NotoSans_SemiBold, NotoSans_Bold) and assign them the proper FontWeight (Medium, SemiBold, Bold) instead of only Normal so headings/buttons use authentic weights; ensure the Font(...) entries for the Latin family in the same file mirror the actual available ttf/otf weights and then verify the app typography styles that reference these weights render as expected.
60-75: Consider using representative glyphs instead of whitespace for font preloading.The current whitespace-based approach in this font fallback preloader may not reliably trigger font loading in Skia, as space glyphs are typically optimized away during rendering. While the invisible box ensures no visibility impact, replacing whitespace with representative script characters (e.g., "अ" for Devanagari, "অ" for Bengali) would better guarantee that each font is fully loaded and cached before needed as a fallback.
Suggested refactor
- val fonts = listOf( - FontFamily(Font(Res.font.NotoSansDevanagari_Regular, FontWeight.Normal)), - FontFamily(Font(Res.font.NotoSansBengali_Regular, FontWeight.Normal)), - FontFamily(Font(Res.font.NotoSansKannada_Regular, FontWeight.Normal)), - FontFamily(Font(Res.font.NotoSansTelugu_Regular, FontWeight.Normal)), - FontFamily(Font(Res.font.NotoSansArabic_Regular, FontWeight.Normal)), - FontFamily(Font(Res.font.NotoSansKhmer_Regular, FontWeight.Normal)), - FontFamily(Font(Res.font.NotoSansMyanmar_Regular, FontWeight.Normal)), - FontFamily(Font(Res.font.NotoSansMalayalam_Regular, FontWeight.Normal)), - ) - fonts.forEach { font -> + val fonts = listOf( + FontFamily(Font(Res.font.NotoSansDevanagari_Regular, FontWeight.Normal)) to "अ", + FontFamily(Font(Res.font.NotoSansBengali_Regular, FontWeight.Normal)) to "অ", + FontFamily(Font(Res.font.NotoSansKannada_Regular, FontWeight.Normal)) to "ಅ", + FontFamily(Font(Res.font.NotoSansTelugu_Regular, FontWeight.Normal)) to "అ", + FontFamily(Font(Res.font.NotoSansArabic_Regular, FontWeight.Normal)) to "ا", + FontFamily(Font(Res.font.NotoSansKhmer_Regular, FontWeight.Normal)) to "ខ", + FontFamily(Font(Res.font.NotoSansMyanmar_Regular, FontWeight.Normal)) to "က", + FontFamily(Font(Res.font.NotoSansMalayalam_Regular, FontWeight.Normal)) to "അ", + ) + fonts.forEach { (font, sample) -> Text( - text = " ", + text = sample, fontFamily = font, ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/designsystem/src/commonMain/kotlin/org/mifos/mobile/core/designsystem/theme/Type.kt` around lines 60 - 75, The preloader uses a space character which may not force Skia to load glyphs; update the Box preloading block that builds the fonts list (the FontFamily(Font(Res.font...)) entries) and the Text call so each Text uses a representative script glyph for that font (e.g., "अ" for Devanagari, "অ" for Bengali, etc.) instead of " "; keep the Box invisible (preserve Modifier.size(0.dp) or equivalent) so there's no UI impact, and ensure each Text is associated with the correct FontFamily from the fonts list so each font is actually loaded/cached for fallback.core/designsystem/src/commonMain/kotlin/org/mifos/mobile/core/designsystem/theme/Theme.kt (1)
287-287: Scope the font fallback preloader to Web-only execution.Line 287 runs
FontFallbackPreloader()on all targets, but the workaround comment inType.ktexplicitly targets Compose Web (Wasm/JS). Consider gating this throughexpect/actual(no-op on native targets) to avoid unnecessary composition work.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/designsystem/src/commonMain/kotlin/org/mifos/mobile/core/designsystem/theme/Theme.kt` at line 287, Theme.kt currently invokes FontFallbackPreloader() across all platforms though the workaround in Type.kt is Web-specific; create an expect function (e.g., expect fun PreloadFontFallbacks()) in common and replace the direct FontFallbackPreloader() call in Theme.kt with PreloadFontFallbacks(), then implement actuals: in the Compose Web (Wasm/JS) actual call FontFallbackPreloader(), and on native/other targets provide a no-op implementation (or an empty `@Composable` actual) so non-Web targets do not run the fallback preloader.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@core/designsystem/src/commonMain/kotlin/org/mifos/mobile/core/designsystem/theme/Theme.kt`:
- Around line 294-303: The LocalTextStyle override placed outside
KptMaterialTheme is being shadowed by MaterialTheme composed inside
KptMaterialTheme; move the CompositionLocalProvider(LocalTextStyle provides
TextStyle(fontFamily = fontFamily)) into the body of KptMaterialTheme (merge it
with the existing LocalTextStyle provided there) so that the custom fontFamily
is applied after MaterialTheme is set, ensuring descendants see the merged
TextStyle; update the Theme.kt usage so KptMaterialTheme(theme = theme, content
= { CompositionLocalProvider(LocalTextStyle provides
currentLocalTextStyle.merge(TextStyle(fontFamily = fontFamily))) { content() }
}) referencing CompositionLocalProvider, LocalTextStyle, TextStyle, and
KptMaterialTheme.
---
Nitpick comments:
In
`@core/designsystem/src/commonMain/kotlin/org/mifos/mobile/core/designsystem/theme/Theme.kt`:
- Line 287: Theme.kt currently invokes FontFallbackPreloader() across all
platforms though the workaround in Type.kt is Web-specific; create an expect
function (e.g., expect fun PreloadFontFallbacks()) in common and replace the
direct FontFallbackPreloader() call in Theme.kt with PreloadFontFallbacks(),
then implement actuals: in the Compose Web (Wasm/JS) actual call
FontFallbackPreloader(), and on native/other targets provide a no-op
implementation (or an empty `@Composable` actual) so non-Web targets do not run
the fallback preloader.
In
`@core/designsystem/src/commonMain/kotlin/org/mifos/mobile/core/designsystem/theme/Type.kt`:
- Around line 39-47: The font registration in Type.kt currently only adds
Regular files (using Font(..., FontWeight.Normal) for Res.font.NotoSans_*),
which forces synthetic weights; update the Font declarations to register real
weight variants for the base Latin family (e.g., Res.font.NotoSans_Medium,
NotoSans_SemiBold, NotoSans_Bold) and assign them the proper FontWeight (Medium,
SemiBold, Bold) instead of only Normal so headings/buttons use authentic
weights; ensure the Font(...) entries for the Latin family in the same file
mirror the actual available ttf/otf weights and then verify the app typography
styles that reference these weights render as expected.
- Around line 60-75: The preloader uses a space character which may not force
Skia to load glyphs; update the Box preloading block that builds the fonts list
(the FontFamily(Font(Res.font...)) entries) and the Text call so each Text uses
a representative script glyph for that font (e.g., "अ" for Devanagari, "অ" for
Bengali, etc.) instead of " "; keep the Box invisible (preserve
Modifier.size(0.dp) or equivalent) so there's no UI impact, and ensure each Text
is associated with the correct FontFamily from the fonts list so each font is
actually loaded/cached for fallback.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f3f7df89-cb2a-4676-b459-3361fe070bcf
⛔ Files ignored due to path filters (18)
core/designsystem/src/commonMain/composeResources/font/NotoSans-Regular.ttfis excluded by!**/*.ttfcore/designsystem/src/commonMain/composeResources/font/NotoSansArabic-Regular.ttfis excluded by!**/*.ttfcore/designsystem/src/commonMain/composeResources/font/NotoSansBengali-Regular.ttfis excluded by!**/*.ttfcore/designsystem/src/commonMain/composeResources/font/NotoSansDevanagari-Regular.ttfis excluded by!**/*.ttfcore/designsystem/src/commonMain/composeResources/font/NotoSansKannada-Regular.ttfis excluded by!**/*.ttfcore/designsystem/src/commonMain/composeResources/font/NotoSansKhmer-Regular.ttfis excluded by!**/*.ttfcore/designsystem/src/commonMain/composeResources/font/NotoSansMalayalam-Regular.ttfis excluded by!**/*.ttfcore/designsystem/src/commonMain/composeResources/font/NotoSansMyanmar-Regular.ttfis excluded by!**/*.ttfcore/designsystem/src/commonMain/composeResources/font/NotoSansTelugu-Regular.ttfis excluded by!**/*.ttfcore/designsystem/src/commonMain/composeResources/font/inter_black.ttfis excluded by!**/*.ttfcore/designsystem/src/commonMain/composeResources/font/inter_bold.ttfis excluded by!**/*.ttfcore/designsystem/src/commonMain/composeResources/font/inter_extra_bold.ttfis excluded by!**/*.ttfcore/designsystem/src/commonMain/composeResources/font/inter_extra_light.ttfis excluded by!**/*.ttfcore/designsystem/src/commonMain/composeResources/font/inter_light.ttfis excluded by!**/*.ttfcore/designsystem/src/commonMain/composeResources/font/inter_medium.ttfis excluded by!**/*.ttfcore/designsystem/src/commonMain/composeResources/font/inter_regular.ttfis excluded by!**/*.ttfcore/designsystem/src/commonMain/composeResources/font/inter_semi_bold.ttfis excluded by!**/*.ttfcore/designsystem/src/commonMain/composeResources/font/inter_thin.ttfis excluded by!**/*.ttf
📒 Files selected for processing (2)
core/designsystem/src/commonMain/kotlin/org/mifos/mobile/core/designsystem/theme/Theme.ktcore/designsystem/src/commonMain/kotlin/org/mifos/mobile/core/designsystem/theme/Type.kt
|
Added fonts (only Regular variants): NotoSans-Regular.ttf Why were only Regular weights added for these fonts, and not other variants like Medium, Bold, etc.? Also, have you checked other screens other than the language selection screen? Please check the screens on remaining platforms. The font directory belongs to commonMain, so we should check and verify if they are compatible across all screens and platforms. |
niyajali
left a comment
niyajali
left a comment
There was a problem hiding this comment.
Why you're updating the whole font in the apps. Consider discuss with maintainers of the approaches and fixes before sending a PR on these kind of changes
@niyajali sir i have discussed with rajan sir in todays standup for this |
|
@Aryan-Baglane okay, what's issue with inter font? See as you might understanding you're changing the font of the application (designsystem) so all the platforms app will use the new font and does they looks similar and good in all screens? |
@niyajali To solve this, we switched to Noto Sans, which provides comprehensive Unicode coverage across multiple languages. This ensures that all supported languages render correctly. Since this change is applied at the design system level, it affects all platforms, but the visual difference between Inter and Noto Sans is minimal as both are modern sans-serif fonts designed for UI readability. |
|
Okay, upload videos by running the whole application and navigate to some screens including before and after to see the differences for all platforms that you can run locally. |
@niyajali ** left side is with updated NotoSans Font and right side is with InterFont ** Screen.Recording.2026-04-06.at.11.21.15.PM.mov |
3 participants
Fixes - MM-578
fixes the incorrect language font rendering in KMP web
Please Add Screenshots If there are any UI changes.
Before
After
Please make sure these boxes are checked before submitting your pull request - thanks!
[] Run the static analysis check
./gradlew checkorci-prepush.shto make sure you didn't break anything[] If you have multiple commits please combine them into one commit by squashing them.
Summary by CodeRabbit