-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Bump to dev08 snapshot build. #67
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
Conversation
Changes: * Refactored modifiers to use fluent style throughout * Switched to new use Icon and Image composables * Switch to using IconButton where appropriate * Updated Scaffolding to use IconButton for navigationIcon * Removed vector helpers from sample that are now included in the library * Update typography of code paraghraphs to expand, and avoid setting background on text in code blocks as alpha is not respected – use the background of the Box * Replaced all Container with Box * General code cleanup to remove nesting that can be replaced with modifiers * Moved Modifiers to *first default argument* to follow style * Aligned bullets with baseline of text * Sized bullets based on font scaling * Removed many named arguments that were not required for readibility
JetNews/app/src/main/java/com/example/jetnews/ui/article/ArticleScreen.kt
Show resolved
Hide resolved
JetNews/app/src/main/java/com/example/jetnews/ui/article/ArticleScreen.kt
Outdated
Show resolved
Hide resolved
JetNews/app/src/main/java/com/example/jetnews/ui/article/ArticleScreen.kt
Show resolved
Hide resolved
JetNews/app/src/main/java/com/example/jetnews/ui/article/PostContent.kt
Outdated
Show resolved
Hide resolved
JetNews/app/src/main/java/com/example/jetnews/ui/article/PostContent.kt
Outdated
Show resolved
Hide resolved
JetNews/app/src/main/java/com/example/jetnews/ui/article/ArticleScreen.kt
Show resolved
Hide resolved
JetNews/app/src/main/java/com/example/jetnews/ui/article/PostContent.kt
Outdated
Show resolved
Hide resolved
JetNews/app/src/main/java/com/example/jetnews/ui/article/PostContent.kt
Outdated
Show resolved
Hide resolved
JetNews/app/src/main/java/com/example/jetnews/ui/article/PostContent.kt
Outdated
Show resolved
Hide resolved
JetNews/app/src/main/java/com/example/jetnews/ui/article/PostContent.kt
Outdated
Show resolved
Hide resolved
JetNews/app/src/main/java/com/example/jetnews/ui/article/PostContent.kt
Outdated
Show resolved
Hide resolved
JetNews/app/src/main/java/com/example/jetnews/ui/article/PostContent.kt
Outdated
Show resolved
Hide resolved
JetNews/app/src/main/java/com/example/jetnews/ui/interests/SelectTopicButton.kt
Outdated
Show resolved
Hide resolved
* Swapped Image for Icon in several places * Prefer srcIn * Added note for bugfix w/ link to bug
Updated based on PR notes. PTAL again @manuelvicnt and @nickbutcher |
JetNews/app/src/main/java/com/example/jetnews/ui/article/ArticleScreen.kt
Outdated
Show resolved
Hide resolved
JetNews/app/src/main/java/com/example/jetnews/ui/home/HomeScreen.kt
Outdated
Show resolved
Hide resolved
JetNews/app/src/main/java/com/example/jetnews/ui/home/PostCardTop.kt
Outdated
Show resolved
Hide resolved
JetNews/app/src/main/java/com/example/jetnews/ui/home/PostCardYourNetwork.kt
Outdated
Show resolved
Hide resolved
JetNews/app/src/main/java/com/example/jetnews/ui/home/PostCards.kt
Outdated
Show resolved
Hide resolved
JetNews/app/src/main/java/com/example/jetnews/ui/home/PostCards.kt
Outdated
Show resolved
Hide resolved
* Switch some Icons back to Image that were not really icons * Use ColorFilter.tint to semantically tint vectors * Formatting
Thanks for review! Updated everything except |
@nickbutcher can you check out the icon coloring on the bottom bar? I'm not sure which call to make there, it might be worth checking the design. Here's a screenshot showing both (heart is an Icon and bookmark is an Image) |
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.
LGTM! Thanks
Checked Figma and the icons on the |
* Revert to Modifier.None for default arugments which is the style in dev08
* Use Icon for all Icons, which matches design.
Thanks! All suggested changes in, hitting the button! |
* Bump to dev08. Changes: * Refactored modifiers to use fluent style throughout * Switched to new use Icon and Image composables * Switch to using IconButton where appropriate * Updated Scaffolding to use IconButton for navigationIcon * Removed vector helpers from sample that are now included in the library * Update typography of code paraghraphs to expand, and avoid setting background on text in code blocks as alpha is not respected – use the background of the Box * Replaced all Container with Box * General code cleanup to remove nesting that can be replaced with modifiers * Moved Modifiers to *first default argument* to follow style * Aligned bullets with baseline of text * Sized bullets based on font scaling * Removed many named arguments that were not required for readibility * Added note for bugfix related to font coloring requiring contentColor w/ link to b/143626708 * Use ColorFilter.tint to semantically tint vectors * Continue using Modifier.None for default arugments which is the style in dev08, this is likely to be replaced with `Modifier` in dev09
Changes:
on text in code blocks as alpha is not respected – use the background of the Box