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

Fix #1000 : Update the color of status bar in various screens #1023

Merged
merged 13 commits into from
May 6, 2020

Conversation

SayantanBanerjee16
Copy link
Contributor

@SayantanBanerjee16 SayantanBanerjee16 commented Apr 27, 2020

Explanation

Fixes #1000 :
Step1) Assigned colors from spreadsheets to color.xml
Step2) Created a Utility method where the update of Status Bar color take place
Step3) Called that Utility method from various places as required.

Spreadsheet :

https://docs.google.com/spreadsheets/d/1XphP6nWoUdOZn9pY2kdgfqMWfJjMfKMb98MXM5rp2HY/edit#gid=0
https://xd.adobe.com/view/e8aa4198-3940-47f9-514a-f41cc54457f6-9e9b/

Checklist

  • The PR title starts with "Fix #bugnum: ", followed by a short, clear summary of the changes. (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
  • The PR explanation includes the words "Fixes #bugnum: ..." (or "Fixes part of #bugnum" if the PR only partially fixes an issue).
  • The PR follows the style guide.
  • The PR does not contain any unnecessary auto-generated code from Android Studio.
  • The PR is made from a branch that's not called "develop".
  • The PR is made from a branch that is up-to-date with "develop".
  • The PR's branch is based on "develop" and not on any other branch.
  • The PR is assigned to an appropriate reviewer in both the Assignees and the Reviewers sections.

@SayantanBanerjee16
Copy link
Contributor Author

SayantanBanerjee16 commented Apr 27, 2020

Screenshots (In order of appearance in spread-sheet):

  1. Profile Chooser

profilechooser

  1. Profile Image

profileimage

  1. General Header

generalheader

  1. Slider open

draweropen

  1. PIN Input

PinInput

  1. Walkthrough

walkthrough

  1. Onboarding 1 to 4

onboarding1

onboarding2

onboarding3

onboarding4

@SayantanBanerjee16
Copy link
Contributor Author

@rt4914 @nikitamarysolomanpvt
I am stuck on these files of the spread-sheet in locating them.

- Hint Full Dialog
- Concept Card
- Registration

Can you please help me out. Thanks :)

Copy link
Contributor

@rt4914 rt4914 left a comment

Choose a reason for hiding this comment

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

This is very nice implementation. I have suggested some changes. Also, HintsAnd Solution is missing this change.

Screenshot_1588048991

For viewing hints follow these steps:

  1. Start What is Fraction? exploration.
  2. Click on Continue till you reach your first question.
  3. In first question, select first option twice as it would be an incorrect answer. wait for 10 secs and in you will see blue box on bottom-left, open it, this screen is for HintsAndSolution.

Thanks.

@rt4914
Copy link
Contributor

rt4914 commented Apr 28, 2020

@rt4914 @nikitamarysolomanpvt
I am stuck on these files of the spread-sheet in locating them.

- Hint Full Dialog
- Concept Card
- Registration

Can you please help me out. Thanks :)

I have replied above for Hints.
For concept card you can make ContentCardTestActivity as launcher activity and test but I think it already has a color.
Registration feature is not available yet, so you can leave it.

@rt4914 rt4914 assigned SayantanBanerjee16 and unassigned rt4914 Apr 28, 2020
@nikitamarysolomanpvt
Copy link
Contributor

Explanation

Fixes #1000 :
Step1) Assigned colors from spreadsheets to color.xml
Step2) Created a Utility method where the update of Status Bar color take place
Step3) Called that Utility method from various places as required.

Checklist

  • The PR title starts with "Fix #bugnum: ", followed by a short, clear summary of the changes. (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
  • The PR explanation includes the words "Fixes #bugnum: ..." (or "Fixes part of #bugnum" if the PR only partially fixes an issue).
  • The PR follows the style guide.
  • The PR does not contain any unnecessary auto-generated code from Android Studio.
  • The PR is made from a branch that's not called "develop".
  • The PR is made from a branch that is up-to-date with "develop".
  • The PR's branch is based on "develop" and not on any other branch.
  • The PR is assigned to an appropriate reviewer in both the Assignees and the Reviewers sections.

@SayantanBanerjee16 Please mention the mock link or sheet link in PR for review.

@nikitamarysolomanpvt nikitamarysolomanpvt removed their assignment Apr 28, 2020
Copy link
Contributor

@nikitamarysolomanpvt nikitamarysolomanpvt left a comment

Choose a reason for hiding this comment

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

PTAL on comments of @rt4914.
Please include Kdocs
@rt4914 we may use statusbarcolor in in toolbar view model as well, am waiting for ben's word.

@SayantanBanerjee16
Copy link
Contributor Author

SayantanBanerjee16 commented Apr 28, 2020

PTAL on comments of @rt4914.
Please include Kdocs
@rt4914 we may use statusbarcolor in in toolbar view model as well, am waiting for ben's word.

@rt4914 @nikitamarysolomanpvt Addressed all the comments. Introduced camelCase colorID and Kdoc also.

Hints and Solution Dialogs Screenshot:-
hints

PTAL. Thanks :)

@nikitamarysolomanpvt
Copy link
Contributor

PTAL on comments of @rt4914.
Please include Kdocs
@rt4914 we may use statusbarcolor in in toolbar view model as well, am waiting for ben's word.

@rt4914 @nikitamarysolomanpvt Addressed all the comments. Introduced camelCase colorID and Kdoc also.

Hints and Solution Dialogs Screenshot:-
hints

PTAL. Thanks :)

@SayantanBanerjee16 Please click resolve conversation after addressing comments.

@SayantanBanerjee16
Copy link
Contributor Author

PTAL on comments of @rt4914.
Please include Kdocs
@rt4914 we may use statusbarcolor in in toolbar view model as well, am waiting for ben's word.

@rt4914 @nikitamarysolomanpvt Addressed all the comments. Introduced camelCase colorID and Kdoc also.
Hints and Solution Dialogs Screenshot:-
hints
PTAL. Thanks :)

@SayantanBanerjee16 Please click resolve conversation after addressing comments.

@nikitamarysolomanpvt All the pending conversations are Done and resolved.

Copy link
Contributor

@nikitamarysolomanpvt nikitamarysolomanpvt left a comment

Choose a reason for hiding this comment

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

Please check it in kitkat seems not working.

@nikitamarysolomanpvt nikitamarysolomanpvt removed their assignment Apr 28, 2020
@rt4914 rt4914 assigned SayantanBanerjee16 and unassigned rt4914 Apr 28, 2020
@rt4914
Copy link
Contributor

rt4914 commented Apr 28, 2020

Please check it in kitkat seems not working.

@SayantanBanerjee16 Check this in kitkat and see how much work do we need to fix this. If it can be fixed in this PR itself then its great.

@SayantanBanerjee16
Copy link
Contributor Author

SayantanBanerjee16 commented Apr 28, 2020

Please check it in kitkat seems not working.

@SayantanBanerjee16 Check this in kitkat and see how much work do we need to fix this. If it can be fixed in this PR itself then its great.

@rt4914 @nikitamarysolomanpvt

So change of status bar color can be done on versions above LOLLIPOP and application of dark icon themes can be done from above MARSHMELLO.

For KITKAT, I approached a method from a stack overflow answer using a github library but although it does draw color to system bar, it is getting overlapped with other screen contents.
Shall I revert this KITKAT library code?

Screenshot (115)

Screenshot (116)

@rt4914
Copy link
Contributor

rt4914 commented Apr 30, 2020

Please check it in kitkat seems not working.

@SayantanBanerjee16 Check this in kitkat and see how much work do we need to fix this. If it can be fixed in this PR itself then its great.

@rt4914 @nikitamarysolomanpvt

So change of status bar color can be done on versions above LOLLIPOP and application of dark icon themes can be done from above MARSHMELLO.

For KITKAT, I approached a method from a stack overflow answer using a github library but although it does draw color to system bar, it is getting overlapped with other screen contents.
Shall I revert this KITKAT library code?

Screenshot (115)

Screenshot (116)

Yes, this solution is not appropriate as it is overlapping on other views. You should try some other solutions for this.

@rt4914 rt4914 assigned SayantanBanerjee16 and unassigned rt4914 Apr 30, 2020
@SayantanBanerjee16
Copy link
Contributor Author

@rt4914 Sorry but am unable to find any solution linked to applying color to status bar in Pre- Lollipop versions. I have reverted the previous introduced code for KITKAT.

@rt4914
Copy link
Contributor

rt4914 commented May 4, 2020

@rt4914 Sorry but am unable to find any solution linked to applying color to status bar in Pre- Lollipop versions. I have reverted the previous introduced code for KITKAT.

@nikitamarysolomanpvt PTAL. Thanks.

@rt4914 rt4914 removed their assignment May 4, 2020
@nikitamarysolomanpvt
Copy link
Contributor

@rt4914 Sorry but am unable to find any solution linked to applying color to status bar in Pre- Lollipop versions. I have reverted the previous introduced code for KITKAT.

@SayantanBanerjee16 try https://stackoverflow.com/questions/22192291/how-to-change-the-status-bar-color-in-android/26215844#26215844
You may merge this and try as another PR. @rt4914 Please guide him with the same.

Copy link
Contributor

@nikitamarysolomanpvt nikitamarysolomanpvt left a comment

Choose a reason for hiding this comment

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

LGTM except the pre lollipop (Kitkat) issue

Copy link
Contributor

@rt4914 rt4914 left a comment

Choose a reason for hiding this comment

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

LGTM, also I have filed an issue to track this. #1042

@rt4914 rt4914 merged commit 3a79a4f into oppia:develop May 6, 2020
@SayantanBanerjee16 SayantanBanerjee16 deleted the status-bar-color branch May 6, 2020 09:15
@nikitamarysolomanpvt nikitamarysolomanpvt removed their assignment May 11, 2020
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.

Update the color of status bar in various screens
3 participants