-
Notifications
You must be signed in to change notification settings - Fork 521
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
Fixed #4409 : Dark Mode support for Confetti #4551
Conversation
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.
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.
Thanks @MohitGupta121! I think this looks really nice! Just had one nit on one of the color's names, otherwise the PR LGTM.
Co-authored-by: Ben Henning <[email protected]>
@BenHenning I commits your suggestion changes. Can you please review it now. |
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.
Thanks @MohitGupta121! This LGTM. However, please don't resolve reviewers' conversation threads. To indicate something is done, you just need to reply with "Done" and then the reviewer will close the thread once they confirm that it's been addressed as expected (otherwise it's hard to track requested changes across review passes). Note that this is covered in https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR so I suggest taking another pass on that just make sure that you're familiar with the team's PR process. :)
Oh really sorry actually when I commit new changes it's automatically mark as resolved. As Rajat, in past also told me the same but don't know it's automatically resolved. I take care next time carefully. |
Hmm @MohitGupta121 I've never run into this before. Can you reach out to me if you notice again? I don't think GitHub conversation threads are ever supposed to automatically resolve. |
Sure if I notice it again I let you know about that. But I think I click on the changes suggest and commit from that. Due to this I think that happens as I remember. |
Ah @MohitGupta121 yes. I forgot about that case. Accepting a suggestion will auto-resolve the conversation thread. I suggest, in those cases, to just reopen the thread and reply to it like you would any other comment. |
oh okay, take care of thread opening next time. |
Explanation
Fixed #4409 : Dark Mode support for Confetti
Essential Checklist
For UI-specific PRs only
If your PR includes UI-related changes, then: