Skip to content
This repository has been archived by the owner on Jun 20, 2023. It is now read-only.

Fix exposure logging progress circle inconsistencies. (EXPOSUREAPP-1835) #2297

Merged
merged 15 commits into from
Feb 9, 2021

Conversation

SamuraiKek
Copy link
Contributor

This PR makes the circle progress in line with the iOS one. Adding the x/14 days inside the circle and changing the progress color depending on the state of the exposure logging.

How to test:

  • Go to exposure logging and look at the number of days inside the progress circle. It should look like x/14 days now.
  • Try playing with the exposure logging states. Stop it, turn it back on again. Make sure that the circle is blue while logging is on and grey while it's off.
  • I've removed two layers of includes in the fragment as they were only being used in this screen. Make sure the layout hasn't changed. Try disabling bluetooth and make sure the right cards appear on the screen.

@SamuraiKek SamuraiKek added bug Something isn't working maintainers Tag pull requests created by maintainers labels Feb 5, 2021
@SamuraiKek SamuraiKek added this to the 1.13.0 milestone Feb 5, 2021
@SamuraiKek SamuraiKek requested review from a team February 5, 2021 12:42
@mtwalli
Copy link
Contributor

mtwalli commented Feb 5, 2021

LGTM, tested on S8

device-2021-02-05-154326
device-2021-02-05-154649

@mtwalli mtwalli self-assigned this Feb 5, 2021
mtwalli
mtwalli previously approved these changes Feb 8, 2021
Copy link
Contributor

@mtwalli mtwalli left a comment

Choose a reason for hiding this comment

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

👍🏼

@ralfgehrer ralfgehrer self-assigned this Feb 8, 2021
@ralfgehrer
Copy link
Contributor

Screenshot 2021-02-08 at 13 35 51

Screenshot 2021-02-08 at 13 36 18

The progress indicator on the risk details is not shown - the blue bar is missing.

@SamuraiKek
Copy link
Contributor Author

SamuraiKek commented Feb 8, 2021

The progress indicator on the risk details is not shown - the blue bar is missing.

Forgot about that. I'll add it there as well.

Edit: Addressed your comment. The progress color should now be rendered correctly everywhere, including the risk cards.

@mtwalli
Copy link
Contributor

mtwalli commented Feb 8, 2021

@SamuraiKek after your changes:
Low risk details screen does not show progress in case bluetooth is disabled, while it shows 2/14
device-2021-02-08-174056

@SamuraiKek
Copy link
Contributor Author

SamuraiKek commented Feb 8, 2021

@SamuraiKek after your changes:
Low risk details screen does not show progress in case bluetooth is disabled, while it shows 2/14

Yes, I've intentionally done this because when bluetooth is turned off, the risk calculation becomes restricted. There isn't really a design guidline for this case, maybe we should ask somebody about it? I can quickly change it to the blue tint.
iOS doesn't even have the progress circle in the low risk details screen ¯_(ツ)_/¯

@ralfgehrer, @harambasicluka, what do you think?

@ralfgehrer
Copy link
Contributor

@SamuraiKek after your changes:
Low risk details screen does not show progress in case bluetooth is disabled, while it shows 2/14

Yes, I've intentionally done this because when bluetooth is turned off, the risk calculation becomes restricted. There isn't really a design guidline for this case, maybe we should ask somebody about it? I can quickly change it to the blue tint.
iOS doesn't even have the progress circle in the low risk details screen ¯_(ツ)_/¯

@ralfgehrer, @harambasicluka, what do you think?

IMHO, the progress should be shown. Otherwise, the circle does not make much sense, does it?

@SamuraiKek
Copy link
Contributor Author

IMHO, the progress should be shown. Otherwise, the circle does not make much sense, does it?

Alright, the same behaviour was happening when the location is disabled so I'll take care of that too.

@ralfgehrer
Copy link
Contributor

Screenshot 2021-02-09 at 11 05 42

When exposure logging is turned off, the progress indicator shall be dark grey - according to the ticket. In the current implementation, it is gone.

Copy link
Contributor

@mtwalli mtwalli left a comment

Choose a reason for hiding this comment

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

LGTM , tested latest changes on Samsung S8 while location and bluetooth on/off.

@sonarcloud
Copy link

sonarcloud bot commented Feb 9, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

7.1% 7.1% Coverage
0.0% 0.0% Duplication

@ralfgehrer ralfgehrer merged commit 1c009fb into release/1.13.x Feb 9, 2021
@ralfgehrer ralfgehrer deleted the fix/1835-tracing-days-circle branch February 9, 2021 13:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working maintainers Tag pull requests created by maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants