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

Modify the design of the icons in the card section #508

Merged

Conversation

WonJoongLee
Copy link
Contributor

@WonJoongLee WonJoongLee commented Aug 15, 2023

Issue

Overview (Required)

  • Modified the design of the icons in the card section on the timetable detail screen.
  • Icon tint colors are different, change to appropriate color
  • wording tint colors are different, change to appropriate color
  • Change the background color of the card components to the appropriate color because they are different

Links

Screenshot

Before After

@github-actions
Copy link

Hi @WonJoongLee! Codes seem to be unformatted. To resolve this issue, please run ./gradlew spotlessKotlinApply and fix the results of ./gradlew lintDebug.. Thank you for your contribution.

1.dp,
),
),
colors = CardDefaults.cardColors(containerColor = Color(0xFFF2F4F1)),
Copy link
Member

Choose a reason for hiding this comment

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

We might have to use theme color for applying a dark theme and it seems like we can apply it by adding elevation.

image

Surface at elevation +1 becomes Surface Container Low
https://material.io/blog/tone-based-surface-color-m3

The problem is how we can get elevation +1 😓

https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:compose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/ColorScheme.kt;l=478-484;drc=033f1c4e0e01fb2702b8f59520777aadaed46e5a

Copy link
Member

@takahirom takahirom Aug 15, 2023

Choose a reason for hiding this comment

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

You can leave this color as it is like surfaceColorAtElevation(1.dp,) in this PR. In that case please create the issue for this 🙏

Copy link
Contributor Author

@WonJoongLee WonJoongLee Aug 15, 2023

Choose a reason for hiding this comment

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

I think it would be better to leave the color as surfaceColorAtElevation(1.dp,). I modified it at 9db725f .

Also, I made the following issue for the card background color issue. (link) 🙇

@github-actions
Copy link

github-actions bot commented Aug 15, 2023

Test Results

31 tests   31 ✔️  2m 34s ⏱️
  7 suites    0 💤
  7 files      0

Results for commit 9db725f.

♻️ This comment has been updated with latest results.

@github-actions github-actions bot temporarily deployed to deploygate-distribution August 15, 2023 00:56 Inactive
Copy link
Member

@takahirom takahirom left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution and making issue!

@github-actions github-actions bot temporarily deployed to deploygate-distribution August 15, 2023 02:42 Inactive
@takahirom takahirom merged commit 1afc72c into DroidKaigi:main Aug 15, 2023
5 checks passed
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.

Modify the design of the icons in the card section on the timetable detail screen.
2 participants