Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add Golden Tiles samples #450
Add Golden Tiles samples #450
Changes from 4 commits
7eb2e10
37dda0d
0b28918
bbc1842
a94f21b
6ee6acf
c5540f6
57610fe
b324e27
1ed0bc9
3a571f6
481509f
0a8eb41
4add77d
9f2caa7
9531106
0a7c3aa
42188bd
85933b9
0322bfa
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Large diffs are not rendered by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any parameter that could change if used as a real tile is exposed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can/should we add an ext function to whatever receiver this is?
Foo.addIdToImageMapping(String, @DrawableRes Int)
drawableResToImageResource(@DrawableRes Int)
exists in Horologist already. Is theFoo
only used for preview or is it the same code that would be used in a renderer?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice idea.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file will likely be deleted since we're moving away from color resources in Compose-era.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bordering on the don't review for content, but would it be helpful to implement these designs using themes using Colors? then using primary or secondary ChipColors factories?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a comment for this elsewhere. I'm not sure themes for tiles is worth it - as I was implementing all of these, I found it easier to specify colors explicitly.
In a real project, I imagine it'll be even less worth it:
themes aren't directly reusable between Compose/thisI'm not sure what it provides.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The link you provided here to an example project does help show some utility - https://github.com/yschimke/rememberwear/blob/main/wear/src/main/kotlin/com/google/wear/soyted/tile/RememberWearTileRenderer.kt#L116
I think we should not introduce themes for the Golden Tiles from themes because it'll only serve to showcase the overhead without any of the benefit; unlike your RememberTheMilk project which is sharing a theme with the main app, each Golden Tile is distinct and has its own theme.
There's an example of a theme being created in
ComponentAudit.kt
which we can leave as-is / improve, since it's used throughout the whole file (there's no requirements/designs for different colors).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IF we want people to be able to take these tiles and copy and paste them, I think it's useful. Or maybe a library of reference tile designs, "templates" that we provide. But your call.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's leave it for now and we'll revisit if there's feedback. I think these samples should facilitate understanding and we shouldn't necessarily be encouraging copy and pasting.