Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Update modifiers impl to use Node api and suppress Compose lint warnings on forked components #947

Merged
merged 9 commits into from
Feb 8, 2024
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -595,6 +595,7 @@ public fun <T : Any> rememberSwipeableStateFor(
*/
@ExperimentalMaterial3Api
@InternalSparkApi
@Suppress("ComposeModifierComposed") // Fork from M3 swipeable that will be removed once the api is stable
// This is a copy from Material 3 Swipeable until b/163132293 is closed
// Until this is fixed use it when you really cannot use a BottomSheetDialogFragment
public fun <T> Modifier.swipeable(
Expand All @@ -607,7 +608,7 @@ public fun <T> Modifier.swipeable(
thresholds: (from: T, to: T) -> ThresholdConfig = { _, _ -> FixedThreshold(56.dp) },
resistance: ResistanceConfig? = resistanceConfig(anchors.keys),
velocityThreshold: Dp = VelocityThreshold,
): Modifier = composed(
): Modifier = this then composed(
inspectorInfo = debugInspectorInfo {
name = "swipeable"
properties["state"] = state
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
* SOFTWARE.
*/
@file:Suppress("ComposeModifierComposed") // These modifiers will be forked so the
// work will be done when it happens

package com.adevinta.spark.components.placeholder

import androidx.compose.foundation.layout.Arrangement.spacedBy
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,7 @@ public interface TooltipState : BasicTooltipState {
public val transition: MutableTransitionState<Boolean>
}

@Suppress("ComposeModifierComposed") // Fork from M3 Tooltip that will be removed once the api is stable
private fun Modifier.textVerticalPadding(
subheadExists: Boolean,
actionExists: Boolean,
Expand All @@ -458,6 +459,7 @@ private fun Modifier.textVerticalPadding(
}
}

@Suppress("ComposeModifierComposed") // Fork from M3 Tooltip that will be removed once the api is stable
private fun Modifier.animateTooltip(
transition: Transition<Boolean>,
): Modifier = composed(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ package com.adevinta.spark.components.surface
import androidx.compose.runtime.Composable
import androidx.compose.runtime.ProvidableCompositionLocal
import androidx.compose.runtime.ReadOnlyComposable
import androidx.compose.runtime.Stable
import androidx.compose.runtime.staticCompositionLocalOf
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.graphics.compositeOver
Expand Down Expand Up @@ -56,6 +57,7 @@ public val LocalElevationOverlay: ProvidableCompositionLocal<ElevationOverlay?>
* See [LocalElevationOverlay] to provide your own [ElevationOverlay]. You can provide `null`
* to have no ElevationOverlay applied.
*/
@Stable
public fun interface ElevationOverlay {
/**
* Returns the new background [Color] to use, representing the original background [color]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,10 @@ internal object TabGroupDefaults {
* @param currentTabPosition [TabPosition] of the currently selected tab. This is used to
* calculate the offset of the indicator this modifier is applied to, as well as its width.
*/
@Suppress("ComposeModifierComposed") // Fork from M3 Tab that will be removed once the api is stable
internal fun Modifier.tabIndicatorOffset(
currentTabPosition: TabPosition,
): Modifier = composed(
): Modifier = this then composed(
inspectorInfo = debugInspectorInfo {
name = "tabIndicatorOffset"
value = currentTabPosition
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import androidx.compose.material3.IconButtonDefaults
import androidx.compose.material3.IconToggleButtonColors
import androidx.compose.material3.LocalContentColor
import androidx.compose.runtime.Composable
import androidx.compose.runtime.Stable
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
Expand Down Expand Up @@ -79,6 +80,7 @@ import androidx.compose.material3.FilledTonalButton as MaterialButton
/**
* Scope that provide pre-made addons for leading and trailing contents of [TextField].
*/
@Stable
public abstract class AddonScope {

/**
Expand Down
2 changes: 2 additions & 0 deletions spark/src/main/kotlin/com/adevinta/spark/tokens/Font.kt
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
package com.adevinta.spark.tokens

import androidx.compose.runtime.Composable
import androidx.compose.runtime.Immutable
import androidx.compose.runtime.ReadOnlyComposable
import androidx.compose.ui.platform.LocalInspectionMode
import androidx.compose.ui.text.font.Font
Expand Down Expand Up @@ -52,6 +53,7 @@ public fun sparkFontFamily(
/**
* Utility class to handle the change of font family for the SparkTheme.
*/
@Immutable
public class SparkFontFamily(
private val isLegacy: Boolean,
private val useSparkTokensHighlighter: Boolean,
Expand Down
2 changes: 2 additions & 0 deletions spark/src/main/kotlin/com/adevinta/spark/tokens/Layout.kt
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ public object Layout {
/**
* Support wide screen by making the content width max 840dp, centered horizontally.
*/
@Suppress("ComposeModifierComposed") // WindowInsets.systemBars is internally a LocalComposition but we
// can't access it to use it in the Node API.
public fun Modifier.bodyWidth(): Modifier = fillMaxWidth()
.composed {
windowInsetsPadding(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,58 +21,20 @@
*/
package com.adevinta.spark.tools.modifiers

import androidx.compose.material3.minimumInteractiveComponentSize
import androidx.compose.ui.Modifier
import androidx.compose.ui.layout.Measurable
import androidx.compose.ui.layout.MeasureResult
import androidx.compose.ui.layout.MeasureScope
import androidx.compose.ui.node.LayoutModifierNode
import androidx.compose.ui.node.ModifierNodeElement
import androidx.compose.ui.platform.InspectorInfo
import androidx.compose.ui.unit.Constraints
import androidx.compose.ui.unit.DpSize
import androidx.compose.ui.unit.dp
import kotlin.math.roundToInt

/**
* Ensure that the composable has a minimum touch target size
* Reserves at least 48.dp in size to disambiguate touch interactions if the element would measure
* smaller.
*
* Duplicate of Material minimumTouchTargetSize() since it's internal on their side
* https://m3.material.io/foundations/accessible-design/accessibility-basics#28032e45-c598-450c-b355-f9fe737b1cd8
*
* This uses the Material recommended minimum size of 48.dp x 48.dp, which may not the same as the
* system enforced minimum size. The minimum clickable / touch target size (48.dp by default) is
* controlled by the system via ViewConfiguration` and automatically expanded at the touch input layer.
soulcramer marked this conversation as resolved.
Show resolved Hide resolved
*
* This modifier is not needed for touch target expansion to happen. It only affects layout, to make
* sure there is adequate space for touch target expansion.
*/
public fun Modifier.minimumTouchTargetSize(): Modifier = this then MinimumTouchTargetModifier()

private class MinimumTouchTargetModifier : ModifierNodeElement<MinimumTouchTargetModifierNode>() {

override fun create() = MinimumTouchTargetModifierNode()
override fun equals(other: Any?): Boolean = true

override fun hashCode(): Int = 0

override fun update(node: MinimumTouchTargetModifierNode) = Unit

override fun InspectorInfo.inspectableProperties() {
name = "minimumTouchTargetSize"
properties["README"] = "Adds outer padding to measure at least 48.dp (default) in " +
"size to disambiguate touch interactions if the element would measure smaller"
}
}

private class MinimumTouchTargetModifierNode : Modifier.Node(), LayoutModifierNode {

private val minimumTouchTargetSize = DpSize(44.dp, 44.dp)
override fun MeasureScope.measure(
measurable: Measurable,
constraints: Constraints,
): MeasureResult {
val placeable = measurable.measure(constraints)

// Be at least as big as the minimum dimension in both dimensions
val width = maxOf(placeable.width, minimumTouchTargetSize.width.roundToPx())
val height = maxOf(placeable.height, minimumTouchTargetSize.height.roundToPx())

return layout(width, height) {
val centerX = ((width - placeable.width) / 2f).roundToInt()
val centerY = ((height - placeable.height) / 2f).roundToInt()
placeable.place(centerX, centerY)
}
}
}
public fun Modifier.minimumTouchTargetSize(): Modifier = this then minimumInteractiveComponentSize()
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,14 @@ import androidx.compose.foundation.layout.BoxScope
import androidx.compose.runtime.Composable
import androidx.compose.runtime.Stable
import androidx.compose.ui.Modifier
import androidx.compose.ui.composed
import androidx.compose.ui.draw.drawWithContent
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.graphics.drawscope.ContentDrawScope
import androidx.compose.ui.node.CompositionLocalConsumerModifierNode
import androidx.compose.ui.node.DrawModifierNode
import androidx.compose.ui.node.ModifierNodeElement
import androidx.compose.ui.node.currentValueOf
import androidx.compose.ui.platform.InspectorInfo
import androidx.compose.ui.unit.Dp
import androidx.compose.ui.unit.dp
import com.adevinta.spark.LocalHighlightComponents
Expand All @@ -37,14 +42,31 @@ import com.adevinta.spark.LocalHighlightComponents
* A [Modifier.Element] that adds a draw layer to identify spark components easily.
*/
@Stable
internal fun Modifier.sparkUsageOverlay(overlayColor: Color = Color.Red): Modifier = composed {
this then if (LocalHighlightComponents.current) {
Modifier.drawWithContent {
drawContent()
internal fun Modifier.sparkUsageOverlay(overlayColor: Color = Color.Red): Modifier =
this then SparkUsageOverlayElement(overlayColor)

private class SparkUsageOverlay(
var overlayColor: Color = Color.Red,
) : Modifier.Node(),
DrawModifierNode,
CompositionLocalConsumerModifierNode {
override fun ContentDrawScope.draw() {
drawContent()
if (currentValueOf(LocalHighlightComponents)) {
drawRect(color = overlayColor, alpha = 0.5f)
}
} else {
Modifier
}
}

private class SparkUsageOverlayElement(val overlayColor: Color) : ModifierNodeElement<SparkUsageOverlay>() {
override fun create() = SparkUsageOverlay(overlayColor)
override fun update(node: SparkUsageOverlay) {
node.overlayColor = overlayColor
}
override fun hashCode() = System.identityHashCode(this)
override fun equals(other: Any?) = (other === this)
override fun InspectorInfo.inspectableProperties() {
name = "Spark Component"
}
}

Expand Down
3 changes: 1 addition & 2 deletions spark/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,7 @@
<string name="spark_user_avatar_content_description">Profile photo</string>

<!--Badge-->
<string name="spark_exceed_max_badge_number_a11y" tools:ignore="PluralsCandidate"
description="Plural form content description for when the number of new notifications exceeds a maximum count[CHAR_LIMIT=NONE]">More than <xliff:g example="999" id="maximum number">%1$d</xliff:g> new notifications</string>
<string name="spark_exceed_max_badge_number_a11y" description="Plural form content description for when the number of new notifications exceeds a maximum count[CHAR_LIMIT=NONE]">More than <xliff:g example="999" id="maximum number">%1$d</xliff:g> new notifications</string>
Fixed Show fixed Hide fixed
<string name="spark_badge_numberless_a11y" description="Content description for new notification (no number count) [CHAR_LIMIT=NONE]">New notification</string>
<plurals name="spark_badge_a11y" description="Plural form content description for number of new notifications [CHAR_LIMIT=NONE]">
<item quantity="one"><xliff:g id="count">%d</xliff:g> new notification</item>
Expand Down
Loading