Skip to content

Commit

Permalink
UI: Refactor Text component and typography handling (#585)
Browse files Browse the repository at this point in the history
### TL;DR
Improved text style handling and simplified Text component parameters.

### What changed?
- Removed redundant `fontFamily` parameter from Text component overload
- Added explicit handling of `LocalTextColor` with fallback to theme's `onSurface` color
- Introduced separate `textStyle` and `textColor` variables for clearer style composition
- Modified style merging to use `copy()` for more explicit property updates

### Why make this change?

When the Text API is used inside a Button, it will provide a composition local style for text and color, which should be respected by the Text component. However, Text can also be used independently, requiring a style parameter. If we provide a style parameter, users might misuse it within a Button. To address this, we are merging and giving higher priority to the composition local text and color styles, then falling back to the provided parameters. This ensures consistency and prevents misuse.
  • Loading branch information
ksharma-xyz authored Feb 6, 2025
1 parent ad8989d commit 9ec4897
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ fun Text(
textAlign: TextAlign = TextAlign.Start,
maxLines: Int = Int.MAX_VALUE,
overflow: TextOverflow = if (maxLines == Int.MAX_VALUE) TextOverflow.Clip else TextOverflow.Ellipsis,
fontFamily: FontFamily? = null,
onTextLayout: ((TextLayoutResult) -> Unit)? = null,
) {
Text(
Expand All @@ -37,7 +36,6 @@ fun Text(
textAlign = textAlign,
maxLines = maxLines,
overflow = overflow,
fontFamily = fontFamily,
onTextLayout = onTextLayout,
)
}
Expand All @@ -55,11 +53,14 @@ fun Text(
onTextLayout: ((TextLayoutResult) -> Unit)? = null,
) {
val contentAlpha = LocalContentAlpha.current
val textStyle = style.merge(LocalTextStyle.current)
val textColor: Color = LocalTextColor.current
.takeIf { it != Color.Unspecified } ?: color ?: KrailTheme.colors.onSurface

BasicText(
text = text,
style = style.merge(
color = color?.copy(alpha = contentAlpha)
?: LocalTextColor.current.copy(alpha = contentAlpha),
style = textStyle.copy(
color = textColor.copy(alpha = contentAlpha),
textAlign = textAlign,
fontFamily = fontFamily,
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package xyz.ksharma.krail.taj.theme

import androidx.compose.runtime.Immutable
import androidx.compose.runtime.staticCompositionLocalOf
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.text.TextStyle
import androidx.compose.ui.text.font.FontFamily
import androidx.compose.ui.text.font.FontWeight
Expand Down

0 comments on commit 9ec4897

Please sign in to comment.