Conversation
There was a problem hiding this comment.
Pull request overview
Introduces element-specific color “keys” (resource names) so SDK consumers can override colors per UI element, and wires one of these element overrides into the Compose GiniTheme for the Skonto section.
Changes:
- Added new
element_colors.xmlwith element-level color resources (e.g., camera title, Skonto section title). - Updated camera fragment layout to use the new
gc_camera_title_colorresource key. - Extended Compose theming (
GiniColorScheme) with askontoSectiongroup sourced from a newGiniColorElementsPrimitives, and switched the Bank SDK Skonto title to use it.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| capture-sdk/sdk/src/main/res/values/element_colors.xml | Adds element-level color resources to be overridden by consumers |
| capture-sdk/sdk/src/main/res/layout/gc_fragment_camera.xml | Uses gc_camera_title_color instead of a primitive color directly |
| capture-sdk/sdk/src/main/java/net/gini/android/capture/ui/theme/colors/GiniColorPalette.kt | Extends GiniColorScheme and maps element primitives into light/dark schemes |
| capture-sdk/sdk/src/main/java/net/gini/android/capture/ui/theme/colors/GiniColorElementsPrimitives.kt | Introduces resource-to-Compose bridge for element colors |
| capture-sdk/sdk/src/main/java/net/gini/android/capture/ui/theme/GiniTheme.kt | Builds and passes element primitives into the Compose color scheme |
| bank-sdk/sdk/src/main/java/net/gini/android/bank/sdk/capture/skonto/colors/section/SkontoSectionColors.kt | Defaults Skonto title color to the new element-specific scheme color |
| bank-sdk/example-app/src/main/res/values/colors.xml | Demonstrates overriding the new element-level color resources |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| val skontoSection: SkontoSection = SkontoSection() | ||
|
|
||
| ) { | ||
|
|
There was a problem hiding this comment.
GiniColorScheme is a public data class, and adding the new skontoSection constructor parameter changes the generated constructor/copy/componentN signatures. This is a binary-incompatible API change for any already-compiled consumers of the published SDK. To avoid breaking clients, consider a non-constructor property, or provide a compatibility secondary constructor matching the previous signature (or confirm this ships only with a major version bump).
| // Binary-compatibility secondary constructor matching the previous signature (without skontoSection) | |
| constructor( | |
| background: Background = Background(), | |
| bottomBar: BottomBar = BottomBar(), | |
| topAppBar: TopAppBar = TopAppBar(), | |
| placeholder: Placeholder = Placeholder(), | |
| text: Text = Text(), | |
| card: Card = Card(), | |
| badge: Badge = Badge(), | |
| button: Button = Button(), | |
| buttonOutlined: ButtonOutlined = ButtonOutlined(), | |
| progressBarButton: ProgressBarButton = ProgressBarButton(), | |
| textField: TextField = TextField(), | |
| toggles: Toggles = Toggles(), | |
| dialogs: Dialogs = Dialogs(), | |
| icons: Icons = Icons(), | |
| datePicker: DatePicker = DatePicker(), | |
| checkbox: Checkbox = Checkbox(), | |
| contextMenu: ContextMenu = ContextMenu(), | |
| logo: Logo = Logo(), | |
| ) : this( | |
| background = background, | |
| bottomBar = bottomBar, | |
| topAppBar = topAppBar, | |
| placeholder = placeholder, | |
| text = text, | |
| card = card, | |
| badge = badge, | |
| button = button, | |
| buttonOutlined = buttonOutlined, | |
| progressBarButton = progressBarButton, | |
| textField = textField, | |
| toggles = toggles, | |
| dialogs = dialogs, | |
| icons = icons, | |
| datePicker = datePicker, | |
| checkbox = checkbox, | |
| contextMenu = contextMenu, | |
| logo = logo, | |
| skontoSection = SkontoSection(), | |
| ) |
| } | ||
| @Immutable | ||
| data class SkontoSection( | ||
| val skontoSectionTitleTextColor: Color = Color.Unspecified, |
There was a problem hiding this comment.
The property name skontoSectionTitleTextColor inside SkontoSection is redundant/inconsistent with the rest of GiniColorScheme (e.g., Text.primary, Logo.tint). Renaming it to something like titleTextColor would avoid the skontoSection.skontoSectionTitleTextColor repetition and make the API easier to read.
| val skontoSectionTitleTextColor: Color = Color.Unspecified, | |
| val titleTextColor: Color = Color.Unspecified, |
| <resources> | ||
|
|
||
| <color name="gc_camera_title_color">@color/gc_light_01</color> | ||
| <color name="skonto_section_title_text_color">@color/gc_dark_02</color> |
There was a problem hiding this comment.
skonto_section_title_text_color defaults to @color/gc_dark_02, but in the dark Compose color scheme the Skonto section title is rendered on a card with containerColor = dark02 (see GiniColorScheme.card.container). This results in very low contrast/unreadable text in dark mode. Consider providing a dark-mode default (e.g., values-night/element_colors.xml mapping it to a light color) or selecting different defaults in giniLightColorScheme vs giniDarkColorScheme while still keeping the override hook.
| <color name="skonto_section_title_text_color">@color/gc_dark_02</color> | |
| <color name="skonto_section_title_text_color">@color/gc_light_01</color> |
| /** | ||
| * Bridge between old way of defining colors by overridden resources and Compose | ||
| * | ||
| * This function will define color primitives based on resources value at res/colors.xml | ||
| */ | ||
| internal fun buildColorPrimitivesBasedOnResources(context: Context) = GiniColorElementsPrimitives( | ||
| skontoSectionTitleTextColor = Color(context.getColor(R.color.skonto_section_title_text_color)), | ||
| ) |
There was a problem hiding this comment.
The KDoc for buildColorPrimitivesBasedOnResources() says the values come from res/colors.xml, but these element overrides are defined in res/values/element_colors.xml (and may also be overridden elsewhere). Updating the comment would prevent confusion for anyone trying to override the resources.
|
|
Atruvia supports 25–30 color themes for their banking customers. They currently manually override colors for multiple UI components. They requested more (ideally all) components be color-customizable to speed up publishing the photo payment module.
This is my proposal for providing color keys per element.
Transition to Element-Specific Color Keys (XML)
In commit c0e5abe, the project moved away from using generic color primitives directly in layouts. Previously, UI elements like the camera title in gc_fragment_camera.xml were hardcoded to use general colors like @color/gc_light_01.
The update introduced element_colors.xml, which defines semantic color keys such as gc_camera_title_color. This allows developers to theme specific UI elements without affecting others that might share the same base color. For example, changing the camera title color no longer requires changing the global gc_light_01 value, providing much better flexibility for white-labeling and customization.
Integration with Jetpack Compose
In commit 76943d8, this semantic approach was extended to the Compose-based elements. A new class, GiniColorElementsPrimitives, was introduced to act as a bridge. It reads these new semantic XML color keys and maps them into the Compose GiniColorScheme.
Key technical changes include:
• GiniColorElementsPrimitives: A new data class that holds element-specific colors (e.g., skontoSectionTitleTextColor) and initializes them from Android resources.
• Theme Expansion: The GiniColorScheme and GiniTheme were updated to include a SkontoSection color group. This ensures that even the most specific UI components (like the Skonto section titles) are now part of the unified theme state.
• Dynamic Updating: The giniLightColorScheme and giniDarkColorScheme functions now accept these element primitives, ensuring that both light and dark modes respect the new specific color overrides.
How the clients can use:
For clients who are used to the old color mapping: no changes will happen. They can still override our color palette and customize our SDK.
For clients who need more detailed and customizable SDK, they need to override the elements that they need. This still uses our color palette and if clients don’t override some colors, we’ll use the colors from our color palette.