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

feat: add grid tokens in the demo app #149

Draft
wants to merge 26 commits into
base: develop
Choose a base branch
from

Conversation

paulinea
Copy link
Member

No description provided.

Copy link

github-actions bot commented Oct 22, 2024

@paulinea paulinea force-pushed the 122-add-grid-tokens-in-the-demo-app branch from 884eda8 to 6a464b3 Compare October 24, 2024 15:37
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolution of this resource is the same as the xxhdpi version and is different from the xxxhdpi light version, so I suppose this is not the correct resource.

Same comment for il_tokens_grid_max_width.png and il_tokens_grid_min_width.png.

@@ -121,14 +125,14 @@ fun TokenCategoryDetailScreen(tokenCategory: TokenCategory, onSubcategoryClick:
Column(
modifier = Modifier
.weight(1f)
.padding(start = if (isTypographyProperty) OudsSpaceKeyToken.Fixed.None.value else OudsSpaceKeyToken.Fixed.Medium.value)
.padding(start = if (noIllustrationProperty) OudsSpaceKeyToken.Fixed.None.value else OudsSpaceKeyToken.Fixed.Medium.value)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's also possible to remove the padding here and use the horizontalArrangement parameter of the Row to achieve the same effect:

horizontalArrangement = Arrangement.spacedBy(OudsSpaceKeyToken.Fixed.Medium.value)

This allows to get rid of the noIllustrationProperty variable too.


@Composable
fun GridIllustrations() {
Image(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rendering is correct but images could be in a column to ensure this method will still work if we change the container. We could also factorize several properties:

@Composable
fun GridIllustrations() {
    Column(
        modifier = Modifier.padding(horizontal = OudsSpaceKeyToken.Fixed.Medium.value),
        verticalArrangement = Arrangement.spacedBy(OudsSpaceKeyToken.Fixed.Medium.value)
    ) {
        val resourceIds = listOf(
            R.drawable.il_tokens_grid_column_margin,
            R.drawable.il_tokens_grid_min_width,
            R.drawable.il_tokens_grid_max_width
        )
        resourceIds.forEach {
            Image(
                modifier = Modifier
                    .fillMaxWidth()
                    .background(OudsColorKeyToken.OnSurface.value), //TODO use BgEmphasizedPrimary token when available
                painter = painterResource(id = it),
                contentDescription = null
            )
        }
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add grid tokens in the demo app
2 participants