PR feedback driven changes:

1. Extracted spacing logic to a separate function to make it slightly easier to understand 2. Removed `SourceChip` overload that allowed setting both before and after indicators 3. Made implementation function internal and renamed to `SourceChipInternal`
guardian · Dec 10, 2024 · 75d603a · 75d603a
1 parent eb67513
commit 75d603a
Show file tree

Hide file tree

Showing 3 changed files with 177 additions and 137 deletions.
diff --git a/android/sample/src/main/kotlin/com/gu/source/previews/ChipsPreview.kt b/android/sample/src/main/kotlin/com/gu/source/previews/ChipsPreview.kt
@@ -1,6 +1,5 @@
 package com.gu.source.previews
 
-import androidx.compose.foundation.Image
 import androidx.compose.foundation.layout.*
 import androidx.compose.material3.HorizontalDivider
 import androidx.compose.material3.Icon
@@ -15,8 +14,8 @@ import com.gu.source.Source
 import com.gu.source.components.chips.ChipIndicator
 import com.gu.source.components.chips.SourceChip
 import com.gu.source.components.chips.SourceChipSupportingButton
+import com.gu.source.components.chips.SourceMultiSelectChip
 import com.gu.source.daynight.AppColour
-import com.gu.source.icons.Check
 import com.gu.source.icons.Plus
 import com.gu.source.presets.typography.TextSans14
 import com.gu.source.presets.typography.TextSansBold17
@@ -55,16 +54,16 @@ internal fun ChipsPreview(modifier: Modifier = Modifier) {
                         isSelected = isSelected,
                         size = size,
                         onClick = {},
-                        badge = {},
+                        showBadge = false,
                     )
 
                     SourceChip(
                         text = previewText,
                         isSelected = isSelected,
                         size = size,
                         onClick = {},
-                        badge = {},
-                        indicatorBefore = ChipIndicator.Icon.Component {
+                        showBadge = false,
+                        iconOrImage = ChipIndicator.Icon.Component {
                             Icon(
                                 imageVector = Source.Icons.Base.Plus,
                                 contentDescription = null,
@@ -78,33 +77,26 @@ internal fun ChipsPreview(modifier: Modifier = Modifier) {
                         isSelected = isSelected,
                         size = size,
                         onClick = {},
-                        badge = {},
-                        indicatorBefore = ChipIndicator.Image.Component {
-                            Image(
-                                painter = painterResource(R.drawable.marina_hyde),
-                                contentDescription = null,
-                                modifier = it,
-                            )
-                        },
+                        showBadge = false,
+                        iconOrImage = ChipIndicator.Image.Painter(
+                            painter = painterResource(R.drawable.marina_hyde),
+                            contentDescription = null,
+                        ),
                     )
 
-                    SourceChip(
+                    SourceMultiSelectChip(
                         text = previewText,
                         isSelected = isSelected,
                         size = size,
                         onClick = {},
-                        badge = {},
-                        indicatorBefore = ChipIndicator.Image.Painter(
+                        showBadge = false,
+                        iconOrImage = ChipIndicator.Image.Painter(
                             painter = painterResource(R.drawable.marina_hyde),
                             contentDescription = null,
                         ),
-                        indicatorAfter = ChipIndicator.Icon.Vector(
-                            imageVector = Source.Icons.Base.Check,
-                            contentDescription = null,
-                        ),
                     )
 
-                    SourceChip(
+                    SourceMultiSelectChip(
                         text = previewText,
                         isSelected = isSelected,
                         showBadge = true,
@@ -113,9 +105,19 @@ internal fun ChipsPreview(modifier: Modifier = Modifier) {
                         style = SourceChip.Style.Default.copy(
                             badgeColour = AppColour.Unspecified,
                         ),
-                        indicatorAfter = ChipIndicator.Icon.Vector(
-                            imageVector = Source.Icons.Base.Check,
-                            contentDescription = null,
+                        iconOrImage = ChipIndicator.Icon.Vector(
+                            imageVector = Source.Icons.Base.Plus,
+                        ),
+                    )
+
+                    SourceMultiSelectChip(
+                        text = previewText,
+                        isSelected = isSelected,
+                        showBadge = true,
+                        size = size,
+                        onClick = {},
+                        style = SourceChip.Style.Default.copy(
+                            badgeColour = AppColour.Unspecified,
                         ),
                     )
                 }
@@ -145,7 +147,7 @@ internal fun ChipsPreview(modifier: Modifier = Modifier) {
                     size = SourceChip.Size.Medium,
                     onClick = {},
                     style = SourceChip.Style.Default,
-                    indicatorBefore = ChipIndicator.Icon.Painter(
+                    iconOrImage = ChipIndicator.Icon.Painter(
                         painter = painterResource(R.drawable.ic_list),
                     ),
                 )
@@ -164,7 +166,7 @@ internal fun ChipsPreview(modifier: Modifier = Modifier) {
             text = "Follow",
             size = SourceChip.Size.Medium,
             onClick = {},
-            indicatorBefore = ChipIndicator.Icon.Vector(
+            iconOrImage = ChipIndicator.Icon.Vector(
                 imageVector = Source.Icons.Base.Plus,
             ),
         )

diff --git a/android/source/detekt-baseline.xml b/android/source/detekt-baseline.xml
@@ -2,6 +2,6 @@
 <SmellBaseline>
   <ManuallySuppressedIssues/>
   <CurrentIssues>
-    <ID>CognitiveComplexMethod:SourceChip.kt$@OptIn(ExperimentalLayoutApi::class) @PreviewPhoneBothMode @Composable internal fun SourceChipPreview(modifier: Modifier = Modifier)</ID>
+    <ID>CognitiveComplexMethod:SourceChip.kt$private fun getSpacing( size: SourceChip.Size, leadingIndicator: ChipIndicator, hasTrailIndicator: Boolean, hasText: Boolean, ): Spacing</ID>
   </CurrentIssues>
 </SmellBaseline>