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.

Sign up for GitHub

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

Jump to bottom

Issue 220 Fix inverse mode card when system is in dark mode #223

Merged
merged 4 commits into from
Apr 19, 2022
Merged

Issue 220 Fix inverse mode card when system is in dark mode #223

merged 4 commits into from
Apr 19, 2022

Conversation

marcorighini
Copy link
Contributor

@marcorighini marcorighini commented Feb 21, 2022 •
edited
Loading

Showing the same way with system in light mode or dark mode

showkase-dark-mode-fix

@marcorighini marcorighini mentioned this pull request Feb 21, 2022
Closed
Issue 220 - Fix dark mode
f0fa8d0 
Inverse mode string
@marcorighini marcorighini force-pushed the Issue-220-darkmode-fix branch from 1a2e584 to f0fa8d0 Compare February 21, 2022 16:33
@marcorighini
Copy link
Contributor Author

Hii @vinaygaba , do you think this could be included in next release? Thanks!

@marcorighini
Copy link
Contributor Author

@vinaygaba what do you think about this PR? Thanks

@vinaygaba
Copy link
Collaborator

@marcorighini This is definitely a really useful improvement. I'm wondering if it makes sense to make this more deterministic - instead of doing inverse, we should explicitly show the other card variants with UI_MODE_NIGHT_NO set.

@marcorighini
Copy link
Contributor Author

@marcorighini This is definitely a really useful improvement. I'm wondering if it makes sense to make this more deterministic - instead of doing inverse, we should explicitly show the other card variants with UI_MODE_NIGHT_NO set.

@vinaygaba
Yes, for a showcase library makes sense to make the behavior deterministic. I applied the changes, default mode is light and dark mode for the dark mode card in the detail.

vinaygaba
vinaygaba reviewed Apr 14, 2022
View reviewed changes
showkase/src/main/java/com/airbnb/android/showkase/ui/ShowkaseComponentDetailScreen.kt Outdated
import androidx.compose.foundation.layout.size
import androidx.compose.foundation.layout.sizeIn
import androidx.compose.foundation.layout.width
import androidx.compose.foundation.layout.*
Copy link
Collaborator

Choose a reason for hiding this comment

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

Avoid wildcard imports. Might need to update you Android Studio settings since its probably happening due to default values present locally

Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like detekt also flagged this on CI, hence the build failure @marcorighini

vinaygaba
vinaygaba approved these changes Apr 15, 2022
View reviewed changes
Copy link
Collaborator

@vinaygaba vinaygaba left a comment

Choose a reason for hiding this comment

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

Ready to be merged once the build failures are fixed

@marcorighini
Copy link
Contributor Author

@vinaygaba fixed in d8ba947

@vinaygaba vinaygaba merged commit 2b8efb3 into airbnb:master Apr 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vinaygaba vinaygaba vinaygaba approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@marcorighini @vinaygaba