feat: add dimension tokens in the demo app #147

paulinea · 2024-10-21T14:30:38Z

No description provided.

github-actions · 2024-10-21T17:13:52Z

🟢 Netlify deploy for commit `6c8e319` succeeded

Deploy preview: https://6728f6f66dee420cf1aeecfd--ouds-android.netlify.app
Deploy log: https://app.netlify.com/sites/ouds-android/deploys/6728f6f66dee420cf1aeecfd

… of TokenProperty

Factorize token properties illustrations

florentmaitre · 2024-10-29T16:26:49Z

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

+
+package com.orange.ouds.theme.tokens
+
+class OudsSizeKeyToken {


This could be an object.

Same comment for OudsSpaceKeyToken.

florentmaitre · 2024-10-29T16:31:11Z

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

+    }
+
+    enum class IconWithText {
+        HeadingExtraLargeSizeShort,


What is the rule to sort enum values?

If it is sorted according to the size, then the variant (ExtraLarge, etc...) is sorted from largest to smallest, but the scale (SizeShort, etc...) is sorted from smallest to largest.

florentmaitre · 2024-10-29T16:48:54Z

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

+        Tall,
+        Taller,
+        Tallest,
+        WithIconNone,


I am not 100% sure it's better, but we could also put WithIcon and WithArrow values into nested enum classes:

enum class PaddingInline { None, Shorter, Short, Medium, Tall, Taller, Tallest; enum class WithIcon { None, Shortest, Shorter, Short, Medium, Tall, Taller, Tallest } enum class WithArrow { None, Shortest, Shorter, Short, Medium, Tall, Taller, Tallest, } }

In that case we must define additional fromToken methods:

@Stable fun OudsSpaces.fromToken(token: OudsSpaceKeyToken.PaddingInline.WithIcon): Dp { return when (token) { OudsSpaceKeyToken.PaddingInline.WithIcon.None -> paddingInlineWithIconNone OudsSpaceKeyToken.PaddingInline.WithIcon.Shortest -> paddingInlineWithIconShortest OudsSpaceKeyToken.PaddingInline.WithIcon.Shorter -> paddingInlineWithIconShorter OudsSpaceKeyToken.PaddingInline.WithIcon.Short -> paddingInlineWithIconShort OudsSpaceKeyToken.PaddingInline.WithIcon.Medium -> paddingInlineWithIconMedium OudsSpaceKeyToken.PaddingInline.WithIcon.Tall -> paddingInlineWithIconTall OudsSpaceKeyToken.PaddingInline.WithIcon.Taller -> paddingInlineWithIconTaller OudsSpaceKeyToken.PaddingInline.WithIcon.Tallest -> paddingInlineWithIconTallest } } @Stable fun OudsSpaces.fromToken(token: OudsSpaceKeyToken.PaddingInline.WithArrow): Dp { return when (token) { OudsSpaceKeyToken.PaddingInline.WithArrow.None -> paddingInlineWithArrowNone OudsSpaceKeyToken.PaddingInline.WithArrow.Shortest -> paddingInlineWithArrowShortest OudsSpaceKeyToken.PaddingInline.WithArrow.Shorter -> paddingInlineWithArrowShorter OudsSpaceKeyToken.PaddingInline.WithArrow.Short -> paddingInlineWithArrowShort OudsSpaceKeyToken.PaddingInline.WithArrow.Medium -> paddingInlineWithArrowMedium OudsSpaceKeyToken.PaddingInline.WithArrow.Tall -> paddingInlineWithArrowTall OudsSpaceKeyToken.PaddingInline.WithArrow.Taller -> paddingInlineWithArrowTaller OudsSpaceKeyToken.PaddingInline.WithArrow.Tallest -> paddingInlineWithArrowTallest } }

We also have to add .value extensions for nested data classes

florentmaitre · 2024-10-29T16:57:09Z

core/src/main/java/com/orange/ouds/core/theme/OudsSpaces.kt


-data class OudsSpacings(
+data class OudsSpaces(
    val fixedNone: Dp,


Should we add nested data classes like what is done in OudsSpaceKeyToken?

data class OudsSpaces( val fixed: Fixed, // ... ) { data class Fixed( val none: Dp, val smash: Dp, val shortest: Dp, val shorter: Dp, val short: Dp, val medium: Dp, val tall: Dp, val taller: Dp, val tallest: Dp, val spacious: Dp, val huge: Dp, val jumbo: Dp ) }

Same comment for OudsSizes.

florentmaitre · 2024-10-29T17:10:42Z

NOTICE.txt

@@ -11,6 +11,8 @@ Any use or displaying shall constitute an infringement under intellectual proper
 app/src/main/res/drawable/ic_border.xml
 app/src/main/res/drawable/ic_component_atom.xml
 app/src/main/res/drawable/ic_design_token_figma.xml
+app/src/main/res/drawable/ic_design_token_figma_no_padding.xml
+app/src/main/res/drawable/ic_dimension.xml


ic_chevron_down.xml is missing, but maybe it is not an Orange resource?

florentmaitre · 2024-10-29T17:40:39Z

app/src/main/res/drawable/ic_design_token_figma_no_padding.xml

@@ -0,0 +1,10 @@
+<vector xmlns:android="http://schemas.android.com/apk/res/android"


Copyright is missing if this is an Orange resource.

Same comment for ic_chevron_down.xml.

florentmaitre · 2024-10-30T10:27:08Z

app/src/main/java/com/orange/ouds/app/ui/tokens/TokenProperty.kt

+                    text = label,
+                    maxLines = 1,
+                    overflow = TextOverflow.Ellipsis,
+                    style = OudsTypographyKeyToken.entries.firstOrNull { it.name == label }?.value.orElse { OudsTypographyKeyToken.BodyStrongLarge.value }


Body and label text styles are all using OudsTypographyKeyToken.BodyStrongLarge:

This can be fixed by removing "Strong" when searching the associated typography token:

style = OudsTypographyKeyToken.entries.firstOrNull { it.name.replace("Strong", "") == label }?.value.orElse { LocalTextStyle.current }

We can also return LocalTextStyle.current as a default value if the typography token cannot be found.

florentmaitre · 2024-10-30T10:39:03Z

app/src/main/java/com/orange/ouds/app/ui/tokens/TokenPropertyIllustration.kt

+    ) {
+        content()
+    }


Suggested change

) {

content()

}

content = content

)

florentmaitre · 2024-10-30T10:57:09Z

app/src/main/java/com/orange/ouds/app/ui/tokens/TokenCategoryDetailScreen.kt

+    is TokenProperty.BorderWidth -> BorderIllustrationBox(width = token.value as Dp)
+    is TokenProperty.BorderRadius -> BorderIllustrationBox(shape = RoundedCornerShape(token.value as Dp))
+    is TokenProperty.BorderStyle -> BorderIllustrationBox(style = token.value as OudsBorderStyle)
+    is TokenProperty.Opacity -> tokenProperty.Illustration(opacity = token.value as Float)


Illustration methods are located into two different files (TokenPropertyIllustration.kt and TokenProperty.kt). Could we move them all in the same file?

florentmaitre · 2024-10-30T11:12:44Z

app/src/main/java/com/orange/ouds/app/ui/tokens/TokenCategoryDetailScreen.kt

+    is TokenProperty.Elevation -> tokenProperty.Illustration(elevation = token.value as Dp)
+    is TokenProperty.SizeIconDecorative -> tokenProperty.Illustration(size = token.value as Dp)
+    is TokenProperty.SizeIconWithLabel -> tokenProperty.Illustration(size = token.value as Dp, token.name)
+    is TokenProperty.SpaceColumnGap, TokenProperty.SpaceFixed, TokenProperty.SpaceScaled -> SpaceIllustrationBox(


Maybe Figma changed meanwhile, but there are several differences with our current UI:

There is a header for each token property.

Scaled spaces are both horizontal and vertical.

Tokens with icon and arrow variants are separated into three sections, maybe this could easily be fixed by splitting the enum as suggested in another comment.

Padding stack with arrow is not displayed in Figma but appears on screen and the arrow missing.

Icons and arrows are missing in column gap with icon, column gap with arrow and column gap with icon.

As discussed together, I created a new issue for the headers: #179

paulinea linked an issue Oct 21, 2024 that may be closed by this pull request

Add dimension tokens in the demo app #120

Open

This comment was marked as resolved.

Sign in to view

paulinea force-pushed the 120-add-dimension-tokens-in-the-demo-app branch from 8b1a027 to cf81b6a Compare October 21, 2024 17:08

paulinea deployed to netlify October 21, 2024 17:10 — with GitHub Actions View deployment

paulinea force-pushed the 120-add-dimension-tokens-in-the-demo-app branch from cf81b6a to 5942aa2 Compare October 22, 2024 08:57

paulinea deployed to netlify October 22, 2024 08:59 — with GitHub Actions View deployment

paulinea deployed to netlify October 22, 2024 09:08 — with GitHub Actions View deployment

paulinea deployed to netlify October 22, 2024 13:37 — with GitHub Actions View deployment

paulinea deployed to netlify October 23, 2024 14:15 — with GitHub Actions View deployment

paulinea force-pushed the 120-add-dimension-tokens-in-the-demo-app branch from 6d3763e to c433916 Compare October 23, 2024 14:15

paulinea deployed to netlify October 23, 2024 14:16 — with GitHub Actions View deployment

paulinea marked this pull request as ready for review October 23, 2024 14:32

paulinea force-pushed the 120-add-dimension-tokens-in-the-demo-app branch from c433916 to 69556aa Compare October 23, 2024 14:34

paulinea deployed to netlify October 23, 2024 14:35 — with GitHub Actions View deployment

paulinea deployed to netlify October 24, 2024 08:38 — with GitHub Actions View deployment

paulinea added 14 commits October 24, 2024 15:29

Add border tokens in the demo app

4384ce7

Review: Change dashedBorder modifier signature

6a862e3

Review: Move defaultIllustrationSize variable in the companion object…

5d0f34a

… of TokenProperty

Add ic_spacing icon

c81a14b

Start adding dimension tokens in demo app

858ae97

Fix subcategories detail display

8254ed5

Reorder OudsSizeIconWithTextKeyToken

457a2b6

Display dimension size tokens

378812b

Display all spacing tokens in the demo app

1d6cbcd

Apply filter before calling private composable

3da3c0d

Reorder properties

057b1c7

Update illustrations for spacing inline with icon

a04f3d3

Rename spacing into space

0b22f2b

Rename strings

7f91d84

paulinea added 2 commits October 24, 2024 15:29

Add WithArrow tokens illustrations

9b76204

Factorize token properties illustrations

Replace hardcoded spaces by tokens

b696f73

paulinea force-pushed the 120-add-dimension-tokens-in-the-demo-app branch from b6626f5 to b696f73 Compare October 24, 2024 13:38

paulinea deployed to netlify October 24, 2024 13:40 — with GitHub Actions View deployment

Factorize some code

79ca7ab

paulinea deployed to netlify October 24, 2024 15:36 — with GitHub Actions View deployment

Improve size and space key tokens organization

3776da3

paulinea deployed to netlify October 25, 2024 10:04 — with GitHub Actions View deployment

florentmaitre reviewed Oct 30, 2024

View reviewed changes

florentmaitre mentioned this pull request Oct 30, 2024

feat: add color tokens #152

Draft

paulinea added 12 commits November 4, 2024 10:54

Review: Use object instead of class

38aa945

Review: Add nested data classes in OudsSizes and OudsSpaces

1f21098

Review: Create nested enum classes for space key tokens

b0beee6

Review: Add ic_chevron_down to NOTICE.txt

8e432f0

Review: Add missing copyrights

a78da36

Review: Update preview

905e4f0

Review: Compute isSubcategory property at runtime

679bb37

Review: Replace SizeIconWithLabel by SizeIconWithText

a15d2c8

Review: Fix body and label text styles

da4effd

Review: Simplify code

26a43ed

Review: Manage token property illustrations in the same file

63adec7

Review: Split tokens with icon/arrow and add corresponding illustrations

6c8e319

paulinea deployed to netlify November 4, 2024 16:28 — with GitHub Actions View deployment

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add dimension tokens in the demo app #147

feat: add dimension tokens in the demo app #147

paulinea commented Oct 21, 2024

This comment was marked as resolved.

github-actions bot commented Oct 21, 2024 •

edited

Loading

florentmaitre Oct 29, 2024

florentmaitre Oct 29, 2024

florentmaitre Oct 29, 2024

paulinea Nov 4, 2024

florentmaitre Oct 29, 2024

florentmaitre Oct 29, 2024

florentmaitre Oct 29, 2024

florentmaitre Oct 30, 2024

florentmaitre Oct 30, 2024

florentmaitre Oct 30, 2024

florentmaitre Oct 30, 2024

paulinea Nov 4, 2024


		package com.orange.ouds.theme.tokens

		class OudsSizeKeyToken {

		@@ -0,0 +1,10 @@
		<vector xmlns:android="http://schemas.android.com/apk/res/android"

feat: add dimension tokens in the demo app #147

Are you sure you want to change the base?

feat: add dimension tokens in the demo app #147

Conversation

paulinea commented Oct 21, 2024

This comment was marked as resolved.

github-actions bot commented Oct 21, 2024 • edited Loading

🟢 Netlify deploy for commit 6c8e319 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

github-actions bot commented Oct 21, 2024 •

edited

Loading

🟢 Netlify deploy for commit `6c8e319` succeeded