feat: create button component #213

florentmaitre · 2024-11-28T14:50:18Z

Note: Please transform - [ ] into - (NA) in the description when things are not applicable

Related issues

Description

Motivation & Context

Types of change

Bug fix (non-breaking which fixes an issue)
New feature (non-breaking change which adds functionality)
Refactoring (non-breaking change)
Breaking change (fix or feature that would change existing functionality)

Previews

Checklist

Contribution

I have read the contributing guidelines

Accessibility

My change follows accessibility good practices

Design

My change respects the design guidelines of Orange Unified Design System

Development

My change follows the developer guide
I have added unit tests to cover my changes (optional)

Documentation

My change introduces changes to the documentation and/or I have updated the documentation accordingly

Checklist (for Core Team only)

Manually test (dark mode, RTL, landscape display, tablet)
Documentation has been updated if relevant
Design review
A11y review
Internal files have been updated if relevant (THIRD_PARTY, NOTICE)
changelog.md has been updated respecting keep a changelog rules and referencing the issue

github-actions · 2024-11-28T14:54:43Z

🟢 Netlify deploy for commit `9d05f30` succeeded

Deploy preview: https://674f49026fa30610e675e0f4--ouds-android.netlify.app
Deploy log: https://app.netlify.com/sites/ouds-android/deploys/674f49026fa30610e675e0f4

paulinea · 2024-12-02T10:41:24Z

core/src/main/java/com/orange/ouds/core/component/button/OudsButton.kt

+@OptIn(ExperimentalMaterial3Api::class)
+private fun OudsButton(
+    nullableIcon: OudsButton.Icon?,
+    nullableText: String?,


As seen together, replace nullableIcon and nullableText respectively by icon and text and remove previewState default value to avoid similar signatures issue.

paulinea · 2024-12-02T10:55:35Z

core/src/main/java/com/orange/ouds/core/component/button/OudsButton.kt

+                .widthIn(min = buttonTokens.sizeMinWidth.dp)
+                .heightIn(min = buttonTokens.sizeMinHeight.dp, max = maxHeight)
+                .border(hierarchy = hierarchy, state = state, shape = shape),
+            enabled = state != OudsButton.State.Disabled && state != OudsButton.State.Loading && state != OudsButton.State.Skeleton,


What do you think about writing this like that:

state !in listOf(OudsButton.State.Disabled, OudsButton.State.Loading, OudsButton.State.Skeleton)

I also added remember to avoid instanciating the list on each recomposition.

paulinea · 2024-12-02T11:09:21Z

core/src/main/java/com/orange/ouds/core/component/button/OudsButton.kt

+                OudsButton.State.Loading,
+                OudsButton.State.Focused -> OudsColorKeyToken.Content.OnAction.Negative
+                OudsButton.State.Disabled -> OudsColorKeyToken.Content.OnAction.Disabled
+                OudsButton.State.Skeleton -> OudsColorKeyToken.Gradient.Skeleton.StartEnd


We always use OudsColorKeyToken.Gradient.Skeleton.StartEnd for Skeleton state, maybe we can factorize it as a skeleton color for OudsButton?

paulinea · 2024-12-02T11:19:28Z

core/src/main/java/com/orange/ouds/core/component/button/OudsButton.kt

+                OudsButton.State.Pressed -> colorContentDefaultPressed
+                OudsButton.State.Loading -> colorContentDefaultLoading
+                OudsButton.State.Disabled -> colorContentDefaultDisabled
+                OudsButton.State.Skeleton -> OudsColorKeyToken.Gradient.Skeleton.StartEnd


By changing the text color of the button when it is in skeleton state, Talkback will probably read the text. Is this what we want in term of a11y?

paulinea · 2024-12-02T11:26:51Z

core/src/main/java/com/orange/ouds/core/component/button/OudsButton.kt

+ */
+@Composable
+fun OudsButton(
+    icon: OudsButton.Icon,


We maybe need a contentDescription parameter for this kind of button (icon only) to handle a11y or something else.

paulinea · 2024-12-02T15:13:47Z

core/src/main/java/com/orange/ouds/core/extensions/ColorExt.kt

+import androidx.compose.ui.graphics.Color
+
+@Composable
+internal fun Color.enabled(enabled: Boolean) = if (enabled) this else copy(alpha = 0.38f)


If I'm not wrong, this alpha value is not defined anywhere in tokens.

This value comes from Material. You are right, we should remove it.

paulinea · 2024-12-02T15:20:43Z

core/src/main/java/com/orange/ouds/core/component/icon/OudsIconDefaults.kt

+import androidx.compose.runtime.Composable
+import androidx.compose.ui.graphics.Color
+
+internal object OudsIconDefaults {


Here we have OudsIconDefaults in a separate file, same thing for OudsIconButtonDefaults but this is not the case for OudsButtonDefault. What do you think about creating an OudsButtonDefaults for consistency?

Good point, this is not consistent. What about moving the defaults into the original file? This is was is done in Material.

paulinea · 2024-12-02T15:39:23Z

core/src/main/java/com/orange/ouds/core/component/button/OudsButton.kt

+                OudsButton.State.Pressed -> OudsColorKeyToken.Action.Negative.Pressed
+                OudsButton.State.Loading -> OudsColorKeyToken.Action.Negative.Loading
+                OudsButton.State.Disabled -> OudsColorKeyToken.Action.Disabled
+                OudsButton.State.Skeleton -> OudsColorKeyToken.Gradient.Skeleton.StartEnd


It seems that semantic tokens are not up-to-date because the negative button in dark mode is not similar to the one on Figma. I took a look and OudsColorKeyToken.Action.Negative.Enabled value is similar in light and dark (scarlet600).

Yes, hopefully this will be fixed by next Tokenator update.

paulinea · 2024-12-02T15:46:08Z

core/src/main/java/com/orange/ouds/core/component/button/OudsButton.kt

+                color = borderColor,
+                shape = shape,
+                insetWidth = OudsBorderKeyToken.Width.Focus.Inset.value,
+                insetColor = OudsColorKeyToken.Border.Focus.Inset.value


The result in Hierarchy.Default and State.Hovered is not correct, a border is missing.

I can't fix it right now because a token is missing, this token will probably be added in next Tokenator update.

Well in fact this is a button token, so I can add it manually.

paulinea · 2024-12-02T16:19:13Z

theme-contract/src/main/java/com/orange/ouds/theme/OudsBorderModifier.kt

+ * @param width The width of the border inset in dp. Use [Dp.Hairline] for a hairline border inset.
+ * @param color The color to paint the border inset with.


Suggested change

* @param width The width of the border inset in dp. Use [Dp.Hairline] for a hairline border inset.

* @param color The color to paint the border inset with.

* @param insetWidth The width of the border inset in dp. Use [Dp.Hairline] for a hairline border inset.

* @param insetColor The color to paint the border inset with.

…rogress indicators

…nds at the same time

…onDefaults into OudsIcon.kt

florentmaitre linked an issue Nov 28, 2024 that may be closed by this pull request

Create component - Button #105

Open

florentmaitre deployed to netlify November 28, 2024 14:51 — with GitHub Actions View deployment

florentmaitre force-pushed the 105-create-button-component branch from b799b30 to 774e610 Compare November 28, 2024 15:06

florentmaitre deployed to netlify November 28, 2024 15:07 — with GitHub Actions View deployment

florentmaitre force-pushed the 105-create-button-component branch from 774e610 to 50f35dd Compare November 28, 2024 15:48

florentmaitre deployed to netlify November 28, 2024 15:49 — with GitHub Actions View deployment

florentmaitre force-pushed the 105-create-button-component branch from 50f35dd to ce2ee68 Compare November 28, 2024 16:53

florentmaitre deployed to netlify November 28, 2024 16:54 — with GitHub Actions View deployment

florentmaitre force-pushed the 105-create-button-component branch from ce2ee68 to 49c3c06 Compare November 29, 2024 15:02

florentmaitre deployed to netlify November 29, 2024 15:03 — with GitHub Actions View deployment

florentmaitre force-pushed the 105-create-button-component branch from 49c3c06 to 34d91ff Compare November 29, 2024 15:04

florentmaitre deployed to netlify November 29, 2024 15:06 — with GitHub Actions View deployment

florentmaitre force-pushed the 105-create-button-component branch from 34d91ff to 988953a Compare November 29, 2024 15:30

florentmaitre deployed to netlify November 29, 2024 15:32 — with GitHub Actions View deployment

florentmaitre marked this pull request as ready for review November 29, 2024 17:12

florentmaitre requested a review from paulinea November 29, 2024 17:12

florentmaitre force-pushed the 105-create-button-component branch from 988953a to 9678d7b Compare December 2, 2024 09:51

florentmaitre deployed to netlify December 2, 2024 09:53 — with GitHub Actions View deployment

paulinea requested changes Dec 2, 2024

View reviewed changes

paulinea mentioned this pull request Dec 3, 2024

Create app structure to demonstrate the first component #210

Closed

florentmaitre and others added 7 commits December 3, 2024 11:00

Update OudsButton according to design specifications

0948e62

Add components navigation system

a5847d5

Add button demo screen with its customization options

d868972

Remove OnResumeEffect and use LifecycleResumeEffect instead

0577600

Remove ComponentDetailScreen

4496cea

Update OudsButton demo with the actual button

6d445b6

OudsButton now supports both determinate and indeterminate circular p…

4d21ddd

…rogress indicators

florentmaitre force-pushed the 105-create-button-component branch from 9678d7b to 4d21ddd Compare December 3, 2024 10:00

florentmaitre deployed to netlify December 3, 2024 10:02 — with GitHub Actions View deployment

florentmaitre added 14 commits December 3, 2024 11:09

Update Android Studio config file

0f421ee

Review: Rename parameters of private OudsButton method

a08ab49

Review: Use a list instead of multiple conditions

c935916

Review: Factorize skeleton color

df52cdd

Review: Remove useless val

64a5f2f

Review: Fix KDoc for OudsButton.Style.Skeleton

ddd848f

Review: Button demo is now displayed for both light and dark backgrou…

203fe08

…nds at the same time

Review: Fix a bug where text was read by TalkBack on a skeleton button

f806329

Add a11y state description for loading button

477f4e1

Review: Add contentDescription parameter to OudsButton.Icon constructors

e584500

Review: Remove useless extension method on Color

ae25e36

Review: Move OudsIconButtonDefaults into OudsIconButton.kt and OudsIc…

f9f821a

…onDefaults into OudsIcon.kt

Review: Add inner border for focus state

f8fc5be

Review: Fix KDoc for outerBorder method

9d05f30

florentmaitre deployed to netlify December 3, 2024 18:03 — with GitHub Actions View deployment

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: create button component #213

feat: create button component #213

florentmaitre commented Nov 28, 2024

github-actions bot commented Nov 28, 2024 •

edited

Loading

paulinea Dec 2, 2024

paulinea Dec 2, 2024

florentmaitre Dec 3, 2024

paulinea Dec 2, 2024

paulinea Dec 2, 2024

paulinea Dec 2, 2024

paulinea Dec 2, 2024

florentmaitre Dec 3, 2024

paulinea Dec 2, 2024

florentmaitre Dec 3, 2024

paulinea Dec 2, 2024

florentmaitre Dec 3, 2024

paulinea Dec 2, 2024

florentmaitre Dec 3, 2024

florentmaitre Dec 3, 2024

paulinea Dec 2, 2024

		* @param width The width of the border inset in dp. Use [Dp.Hairline] for a hairline border inset.
		* @param color The color to paint the border inset with.

feat: create button component #213

Are you sure you want to change the base?

feat: create button component #213

Conversation

florentmaitre commented Nov 28, 2024

Related issues

Description

Motivation & Context

Types of change

Previews

Checklist

Contribution

Accessibility

Design

Development

Documentation

Checklist (for Core Team only)

github-actions bot commented Nov 28, 2024 • edited Loading

🟢 Netlify deploy for commit 9d05f30 succeeded

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Nov 28, 2024 •

edited

Loading

🟢 Netlify deploy for commit `9d05f30` succeeded