Skip to content

Commit

Permalink
Merge pull request #662 from Orange-OpenSource/661-apis-review-fix-in…
Browse files Browse the repository at this point in the history
…consistencies-in-components-apis

661 - APIs review - Fix inconsistencies in components APIs
  • Loading branch information
florentmaitre authored Oct 16, 2023
2 parents 6a84152 + 8e18263 commit d7203c2
Show file tree
Hide file tree
Showing 8 changed files with 53 additions and 52 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,10 @@ fun ComponentSliders() {
) {
val technicalText = if (shouldDisplayValue) OdsComposable.OdsSliderLockups.name else OdsComposable.OdsSlider.name
val steps = if (isStepped) 9 else 0
val leftIconContentDescription = stringResource(id = R.string.component_slider_low_volume)
val leftIcon = if (hasSideIcons) OdsSliderIcon(painterResource(id = R.drawable.ic_volume_status_1), leftIconContentDescription) else null
val rightIconContentDescription = stringResource(id = R.string.component_slider_high_volume)
val rightIcon = if (hasSideIcons) OdsSliderIcon(painterResource(id = R.drawable.ic_volume_status_4), rightIconContentDescription) else null
val startIconContentDescription = stringResource(id = R.string.component_slider_low_volume)
val startIcon = if (hasSideIcons) OdsSliderIcon(painterResource(id = R.drawable.ic_volume_status_1), startIconContentDescription) else null
val endIconContentDescription = stringResource(id = R.string.component_slider_high_volume)
val endIcon = if (hasSideIcons) OdsSliderIcon(painterResource(id = R.drawable.ic_volume_status_4), endIconContentDescription) else null

var sliderPosition by remember { mutableStateOf(0f) }
val valueRange = 0f..100f
Expand All @@ -89,8 +89,8 @@ fun ComponentSliders() {
steps = steps,
valueRange = valueRange,
onValueChange = { sliderPosition = it },
leftIcon = leftIcon,
rightIcon = rightIcon
startIcon = startIcon,
endIcon = endIcon
)
} else {
componentName = OdsComposable.OdsSlider.name
Expand All @@ -99,8 +99,8 @@ fun ComponentSliders() {
steps = steps,
valueRange = valueRange,
onValueChange = { sliderPosition = it },
leftIcon = leftIcon,
rightIcon = rightIcon
startIcon = startIcon,
endIcon = endIcon
)
}

Expand All @@ -114,13 +114,13 @@ fun ComponentSliders() {
lambda("onValueChange")
if (isStepped) stringRepresentation("steps", steps)
if (hasSideIcons) {
classInstance<OdsSliderIcon>("leftIcon") {
classInstance<OdsSliderIcon>("startIcon") {
painter()
contentDescription(leftIconContentDescription)
contentDescription(startIconContentDescription)
}
classInstance<OdsSliderIcon>("rightIcon") {
classInstance<OdsSliderIcon>("endIcon") {
painter()
contentDescription(rightIconContentDescription)
contentDescription(endIconContentDescription)
}
}
})
Expand Down
1 change: 1 addition & 0 deletions changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Changed

- \[Lib\] Update `OdsBottomNavigation`, `OdsChoiceChipsFlowRow`, `OdsSlider` and `OdsSliderLockups` APIs ([#661](https://github.com/Orange-OpenSource/ods-android/issues/661))
- \[Lib\] Review and update technical documentation ([#638](https://github.com/Orange-OpenSource/ods-android/issues/638))

### Fixed
Expand Down
10 changes: 5 additions & 5 deletions docs/components/Chips.md
Original file line number Diff line number Diff line change
Expand Up @@ -215,22 +215,22 @@ outlined by default.

```kotlin
OdsChoiceChipsFlowRow(
value = chipValue,
onValueChange = { value -> chipValue = value },
modifier = Modifier.padding(horizontal = dimensionResource(id = com.orange.ods.R.dimen.spacing_m)),
chips = listOf(
OdsChoiceChip(text = "Choice chip 1", value = 1),
OdsChoiceChip(text = "Choice chip 2", value = 2)
)
),
value = chipValue,
onValueChange = { value -> chipValue = value },
modifier = Modifier.padding(horizontal = dimensionResource(id = com.orange.ods.R.dimen.spacing_m))
)
```

#### OdsChoiceChipsFlowRow API

Parameter | Default&nbsp;value | Description
-- | -- | --
`chips: List<OdsChoiceChip<T>>` | | Chips displayed into the flow row
`value: String` | | Initial value of the choice chips flow row
`onValueChange: (value: T) -> Unit` | | Callback invoked when the value changes. The new value is provided as parameter.
`modifier: Modifier` | `Modifier` | `Modifier` applied to the chips flow row
`chips: List<OdsChoiceChip<T>>` | | Chips displayed into the flow row
{:.table}
2 changes: 1 addition & 1 deletion docs/components/NavigationBottom.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,6 @@ OdsBottomNavigation(

Parameter | Default&nbsp;value | Description
-- | -- | --
`modifier: Modifier` | `Modifier` | `Modifier` applied to the bottom navigation
`items: List<OdsBottomNavigationItem>` | | Items displayed into the bottom navigation
`modifier: Modifier` | `Modifier` | `Modifier` applied to the bottom navigation
{:.table}
24 changes: 12 additions & 12 deletions docs/components/Sliders.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,11 @@ OdsSlider(
value = 20f,
steps = 9,
onValueChange = { doSomething() },
leftIcon = OdsSliderIcon(
startIcon = OdsSliderIcon(
painterResource(id = R.drawable.ic_volume_status_1),
stringResource(id = R.string.component_slider_low_volume)
),
rightIcon = OdsSliderIcon(
endIcon = OdsSliderIcon(
painterResource(id = R.drawable.ic_volume_status_4),
stringResource(id = R.string.component_slider_high_volume)
)
Expand All @@ -85,8 +85,8 @@ Parameter | Default&nbsp;value | Description
`valueRange: ClosedFloatingPointRange<Float>` | `0f..1f` | Range of values that the slider can take. Given `value` will be coerced to this range.
`steps: Int` | `0` | If greater than `0`, specifies the amounts of discrete values, evenly distributed between across the whole value range. If `0`, slider will behave as a continuous slider and allow to choose any value from the range specified. Must not be negative.
`onValueChangeFinished: (() -> Unit)?` | `null` | Callback invoked when value change has ended. This callback shouldn't be used to update the slider value (use `onValueChange` for that), but rather to know when the user has completed selecting a new value by ending a drag or a click.
`leftIcon: OdsSliderIcon?` | `null` | Icon displayed on the left of the slider
`rightIcon: OdsSliderIcon?` | `null` | Icon displayed on the right of the slider
`startIcon: OdsSliderIcon?` | `null` | Icon displayed at the start of the slider
`endIcon: OdsSliderIcon?` | `null` | Icon displayed at the end of the slider
{:.table}

### Continuous lockups slider
Expand Down Expand Up @@ -116,11 +116,11 @@ OdsSliderLockups(
value = 20f,
valueRange = 0f..100f,
onValueChange = { doSomething() },
leftIcon = OdsSliderIcon(
startIcon = OdsSliderIcon(
painterResource(id = R.drawable.ic_volume_status_1),
stringResource(id = R.string.component_slider_low_volume)
),
rightIcon = OdsSliderIcon(
endIcon = OdsSliderIcon(
painterResource(id = R.drawable.ic_volume_status_4),
stringResource(id = R.string.component_slider_high_volume)
)
Expand All @@ -138,8 +138,8 @@ Parameter | Default&nbsp;value | Description
`valueRange: ClosedFloatingPointRange<Float>` | `0f..1f` | Range of values that the slider can take. Given `value` will be coerced to this range.
`steps: Int` | `0` | If greater than `0`, specifies the amounts of discrete values, evenly distributed between across the whole value range. If `0`, slider will behave as a continuous slider and allow to choose any value from the range specified. Must not be negative.
`onValueChangeFinished: (() -> Unit)?` | `null` | Callback invoked when value change has ended. This callback shouldn't be used to update the slider value (use `onValueChange` for that), but rather to know when the user has completed selecting a new value by ending a drag or a click.
`leftIcon: OdsSliderIcon?` | `null` | Icon displayed on the left of the slider
`rightIcon: OdsSliderIcon?` | `null` | Icon displayed on the right of the slider
`startIcon: OdsSliderIcon?` | `null` | Icon displayed at the start of the slider
`endIcon: OdsSliderIcon?` | `null` | Icon displayed at the end of the slider
{:.table}

### Discrete slider
Expand Down Expand Up @@ -174,11 +174,11 @@ OdsSlider(
valueRange = 0f..100f,
steps = 10,
onValueChange = { doSomething() },
leftIcon = OdsSliderIcon(
startIcon = OdsSliderIcon(
painterResource(id = R.drawable.ic_volume_status_1),
stringResource(id = R.string.component_slider_low_volume)
),
rightIcon = OdsSliderIcon(
endIcon = OdsSliderIcon(
painterResource(id = R.drawable.ic_volume_status_4),
stringResource(id = R.string.component_slider_high_volume)
)
Expand Down Expand Up @@ -216,11 +216,11 @@ OdsSliderLockups(
valueRange = 0f..100f,
steps = 10,
onValueChange = { doSomething() },
leftIcon = OdsSliderIcon(
startIcon = OdsSliderIcon(
painterResource(id = R.drawable.ic_volume_status_1),
stringResource(id = R.string.component_slider_low_volume)
),
rightIcon = OdsSliderIcon(
endIcon = OdsSliderIcon(
painterResource(id = R.drawable.ic_volume_status_4),
stringResource(id = R.string.component_slider_high_volume)
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,14 @@ import com.orange.ods.compose.theme.OdsTheme
* See [OdsBottomNavigationItem] for configuration specific to each item, and not the overall
* OdsBottomNavigation component.
*
* @param modifier [Modifier] applied to the bottom navigation.
* @param items List of [OdsBottomNavigationItem] displayed into the bottom navigation.
* @param modifier [Modifier] applied to the bottom navigation.
*/
@Composable
@OdsComposable
fun OdsBottomNavigation(
modifier: Modifier = Modifier,
items: List<OdsBottomNavigationItem>
items: List<OdsBottomNavigationItem>,
modifier: Modifier = Modifier
) {
BottomNavigation(
modifier = modifier,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,19 +39,19 @@ import com.orange.ods.compose.theme.OdsTheme
*
* Note that [OdsChoiceChip] are displayed outlined or filled according to your [OdsTheme] component configuration, outlined by default.
*
* @param chips The list of [OdsChoiceChip] displayed into the chips flow row.
* @param value The initial value of the choice chips flow row.
* @param onValueChange Callback invoked when the value changes. The new value is provided as parameter.
* @param modifier [Modifier] applied to the chips flow row.
* @param chips The list of [OdsChoiceChip] displayed into the chips flow row.
*/
@OptIn(ExperimentalLayoutApi::class)
@Composable
@OdsComposable
fun <T> OdsChoiceChipsFlowRow(
chips: List<OdsChoiceChip<T>>,
value: T,
onValueChange: (value: T) -> Unit,
modifier: Modifier = Modifier,
chips: List<OdsChoiceChip<T>>
modifier: Modifier = Modifier
) {
var selectedChipValue by remember { mutableStateOf(value) }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,8 @@ private const val ActiveTickColorAlpha = 0.4f
* behave as a continuous slider and allow to choose any value from the range specified. Must not be negative.
* @param onValueChangeFinished Callback invoked when value change has ended. This callback shouldn't be used to update
* the slider value (use [onValueChange] for that), but rather to know when the user has completed selecting a new value by ending a drag or a click.
* @param leftIcon [OdsSliderIcon] displayed on the left of the slider.
* @param rightIcon [OdsSliderIcon] displayed on the right of the slider.
* @param startIcon [OdsSliderIcon] displayed at the start of the slider.
* @param endIcon [OdsSliderIcon] displayed at the end of the slider.
*/
@Composable
@OdsComposable
Expand All @@ -84,15 +84,15 @@ fun OdsSlider(
valueRange: ClosedFloatingPointRange<Float> = 0f..1f,
steps: Int = 0,
onValueChangeFinished: (() -> Unit)? = null,
leftIcon: OdsSliderIcon? = null,
rightIcon: OdsSliderIcon? = null
startIcon: OdsSliderIcon? = null,
endIcon: OdsSliderIcon? = null
) {
Row(
modifier = modifier,
verticalAlignment = Alignment.CenterVertically,
horizontalArrangement = Arrangement.spacedBy(dimensionResource(id = R.dimen.spacing_m))
) {
leftIcon?.Content()
startIcon?.Content()
// For the moment we cannot change the height of the slider track (need to check in jetpack compose future versions)
Slider(
value = value,
Expand All @@ -105,7 +105,7 @@ fun OdsSlider(
onValueChangeFinished = onValueChangeFinished,
colors = SliderDefaults.colors(activeTickColor = OdsTheme.colors.surface.copy(alpha = ActiveTickColorAlpha))
)
rightIcon?.Content()
endIcon?.Content()
}
}

Expand Down Expand Up @@ -134,8 +134,8 @@ fun OdsSlider(
* behave as a continuous slider and allow to choose any value from the range specified. Must not be negative.
* @param onValueChangeFinished Callback invoked when value change has ended. This callback shouldn't be used to update
* the slider value (use [onValueChange] for that), but rather to know when the user has completed selecting a new value by ending a drag or a click.
* @param leftIcon [OdsSliderIcon] displayed on the left of the slider.
* @param rightIcon [OdsSliderIcon] displayed on the right of the slider.
* @param startIcon [OdsSliderIcon] displayed at the start of the slider.
* @param endIcon [OdsSliderIcon] displayed at the end of the slider.
*/
@Composable
@OdsComposable
Expand All @@ -147,8 +147,8 @@ fun OdsSliderLockups(
valueRange: ClosedFloatingPointRange<Float> = 0f..1f,
steps: Int = 0,
onValueChangeFinished: (() -> Unit)? = null,
leftIcon: OdsSliderIcon? = null,
rightIcon: OdsSliderIcon? = null
startIcon: OdsSliderIcon? = null,
endIcon: OdsSliderIcon? = null
) {
val labelMinWidth = 32.dp
val sideIconBottomPadding = 12.dp
Expand All @@ -157,7 +157,7 @@ fun OdsSliderLockups(
modifier = modifier,
horizontalArrangement = Arrangement.spacedBy(dimensionResource(id = R.dimen.spacing_xs))
) {
leftIcon?.Content(
startIcon?.Content(
modifier = Modifier
.align(alignment = Alignment.Bottom)
.padding(bottom = sideIconBottomPadding)
Expand Down Expand Up @@ -194,7 +194,7 @@ fun OdsSliderLockups(
onValueChangeFinished = onValueChangeFinished,
)
}
rightIcon?.Content(
endIcon?.Content(
modifier = Modifier
.align(alignment = Alignment.Bottom)
.padding(bottom = sideIconBottomPadding),
Expand Down Expand Up @@ -285,8 +285,8 @@ private fun PreviewOdsSlider(@PreviewParameter(OdsSliderPreviewParameterProvider
value = sliderValue.value,
onValueChange = { sliderValue.value = it },
steps = 9,
leftIcon = if (withIcons) OdsSliderIcon(painterResource(id = R.drawable.ic_crosset_out_eye), "") else null,
rightIcon = if (withIcons) OdsSliderIcon(painterResource(id = R.drawable.ic_eye), "") else null,
startIcon = if (withIcons) OdsSliderIcon(painterResource(id = R.drawable.ic_crosset_out_eye), "") else null,
endIcon = if (withIcons) OdsSliderIcon(painterResource(id = R.drawable.ic_eye), "") else null,
)
}

Expand All @@ -298,8 +298,8 @@ private fun PreviewOdsSliderLockups(@PreviewParameter(OdsSliderPreviewParameterP
value = value,
valueRange = 0f..100f,
onValueChange = { value = it },
leftIcon = if (withIcons) OdsSliderIcon(painterResource(id = R.drawable.ic_crosset_out_eye), "") else null,
rightIcon = if (withIcons) OdsSliderIcon(painterResource(id = R.drawable.ic_eye), "") else null,
startIcon = if (withIcons) OdsSliderIcon(painterResource(id = R.drawable.ic_crosset_out_eye), "") else null,
endIcon = if (withIcons) OdsSliderIcon(painterResource(id = R.drawable.ic_eye), "") else null,
)
}

Expand Down

0 comments on commit d7203c2

Please sign in to comment.