Forms: Add theming support
#362
Conversation
remove creds from test
* added measure and placement constraints for the bottomsheetscaffold * removed transparent top bar :(
…n support. fix some things that broke through merges remove serialization dependency
consume core
…ate object across pause/resume
remember saveable DateTimeFields
date time fixes
forms tests core compliance
… the date time picker state. add a call to dateTimePickerState.setSelection so that even though the state object is remembered, the selection is re-set on recompose.
sorenoid
force-pushed
the
feature-branches/forms
branch
from
6cb93b7 to
10504ef
Compare
kaushikrw
force-pushed
the
kaushik/forms/theming
branch
from
f7e68ed to
f791558
Compare
There was a problem hiding this comment.
@kaushikrw this looks great and fills a valuable need for FeatureForms. I have some very high level suggestions
- provide a dark mode color palette that is applied automatically when in dark mode
- add since tags to all public API
- provide a simple theme for the FeatureForm itself in addition to the individual components of the form.
- this theme could be used for the body of GroupElements as well, perhaps with different defaults.
| // transform placeholder colors into text colors | ||
| val transformPlaceHolderColors = textIsEmpty && !placeHolderIsEmpty | ||
| val transformedFocusedTextColor = if (transformPlaceHolderColors) { | ||
| // if placeholder is visible, make it lighter than the actual input text color |
There was a problem hiding this comment.
this is a nice addition to the TextField defaults. Was this based on an issue in the apollo backlog? I couldn't find one but I remember a mention of lightening placeholder text.
There was a problem hiding this comment.
I don't think we ever created an issue, but this behavior also exists in the pre-custom theme system.
| colors = CardDefaults.cardColors( | ||
| containerColor = colors.headerColor | ||
| ), | ||
| border = BorderStroke(GroupElementDefaults.borderThickness, colors.outlineColor) |
There was a problem hiding this comment.
I see that GroupElementColors.containerColor is also defined and used for the body of the card. I think it would make sense to call the headerColor containerColor so it matches the Card colors property name, and changing the current GroupElementColors.containerColor to bodyColor.
I think ultimately this points out that there is no overall FeatureFormColors and FeatureFormDefaults for the form container, just the elements contained in it. If there were, it would make sense to reuse it with a separate set of defaults for the card's body.
| val TertiaryContainer = PaletteTokens.Tertiary90 | ||
| } | ||
|
|
||
| private object PaletteTokens { |
There was a problem hiding this comment.
I think this needs to check isSystemInDarkTheme() and offer dark mode alternatives/
There was a problem hiding this comment.
I'm not sure this is needed. The DefaultThemeTokens only needs to provide default values for LocalColorScheme and LocalTypography. These defaults are only utilized in internal compose previews. During runtime the actual values are always taken from MaterialTheme based on current light/dark theme.
...kit/featureforms/src/main/java/com/arcgismaps/toolkit/featureforms/theme/FeatureFormTheme.kt
Show resolved
Hide resolved
|
@sorenoid I've updated this based on our new design standard and addressed some of the feedback. Please have another look! |
|
Thanks @sorenoid, merging. |
Summary of changes
Details
It provides a way to customize the color scheme and typography using two classes.
FeatureFormColorSchemeFeatureFormTypoGraphyThese individual classes allow a user to fully customize the color and typography for all the fields independently.
These properties are passed down the compose tree using the
LocalColorSchemeandLocalLocalTypographycomposition local.All these classes provide a
createDefaults()factory method that can only be called from aComposablecontext. ThecreateDefaults()method hydrates the individual properties of these classes fromMaterialTheme. It also provides parameters with default values, so that any developer can quickly glance at whatMaterialThemevalues are being set as defaults.Due to this design, the
FeatureFormColorSchemeandFeatureFormTypoGraphycan only be created from aComposablecontext, since it relies onMaterialThemefor default values. Hence, as a convenience, an object classDefaultThemeTokensis introduced which provides a defaultcolorSchemeandtypographybased on reasonable hardcoded values. These hardcoded defaults are adapted from theMaterial3.lightColorSchemeandMaterialTheme.Typography.Furthermore, the
DefaultThemeTokensare also useful in providing a good default value for theLocalColorSchemeandLocalLocalTypographycomposition local. This is particularly helpful for compose previews.