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

Certificates screen with loading state (EXPOSUREAPP-11502) #4727

Merged
merged 7 commits into from
Jan 26, 2022
Merged

Certificates screen with loading state (EXPOSUREAPP-11502) #4727

merged 7 commits into from
Jan 26, 2022

Conversation

jurajkusnier
Copy link
Contributor

Testing

  • scan many certificates
  • check if the Certificate screen works as expected
  • you should see the loading state if it takes longer to render the page
  • maybe test on older devices or add delay(1000L) on line 40 in PersonOverviewViewModel

Jira Ticket

https://jira-ibs.wbs.net.sap/browse/EXPOSUREAPP-11502

@jurajkusnier jurajkusnier added maintainers Tag pull requests created by maintainers text change PRs with text changes. labels Jan 24, 2022
@jurajkusnier jurajkusnier added this to the 2.18.0 milestone Jan 24, 2022
@jurajkusnier jurajkusnier requested review from a team January 24, 2022 13:36
CV113
CV113 previously approved these changes Jan 24, 2022
View reviewed changes
Copy link
Contributor

@CV113 CV113 left a comment

Choose a reason for hiding this comment

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

UA approved

@LukasLechnerDev LukasLechnerDev self-assigned this Jan 25, 2022
LukasLechnerDev
LukasLechnerDev reviewed Jan 25, 2022
View reviewed changes
...ain/java/de/rki/coronawarnapp/covidcertificate/person/ui/overview/PersonOverviewViewModel.kt
@@ -117,6 +120,11 @@ class PersonOverviewViewModel @AssistedInject constructor(
expirationNotificationService.showNotificationIfStateChanged(ignoreLastCheck = true)
}

sealed class UiState {
object Loading : UiState()
data class Done(val personCertificates: List<PersonCertificatesItem>) : UiState()
Copy link
Contributor

Choose a reason for hiding this comment

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

What about calling the UiState Success instead of Done?

LukasLechnerDev
LukasLechnerDev approved these changes Jan 25, 2022
View reviewed changes
Copy link
Contributor

@LukasLechnerDev LukasLechnerDev left a comment

Choose a reason for hiding this comment

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

1 Optional Comment, otherwise LGTM.

@SamuraiKek SamuraiKek self-assigned this Jan 26, 2022
SamuraiKek
SamuraiKek approved these changes Jan 26, 2022
View reviewed changes
Copy link
Contributor

@SamuraiKek SamuraiKek left a comment

Choose a reason for hiding this comment

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

LGTM.

CV113
CV113 approved these changes Jan 26, 2022
View reviewed changes
Copy link
Contributor

@CV113 CV113 left a comment

Choose a reason for hiding this comment

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

UA approved

@sonarcloud
Copy link

sonarcloud bot commented Jan 26, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

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

27.3% 27.3% Coverage
0.0% 0.0% Duplication

@jurajkusnier jurajkusnier merged commit 11d6ab7 into release/2.18.x Jan 26, 2022
@jurajkusnier jurajkusnier deleted the feature/11502-person_overview_loading_state branch January 26, 2022 14:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@LukasLechnerDev LukasLechnerDev LukasLechnerDev approved these changes

@SamuraiKek SamuraiKek SamuraiKek approved these changes

@CV113 CV113 CV113 approved these changes

Assignees

@LukasLechnerDev LukasLechnerDev

@SamuraiKek SamuraiKek

Labels
maintainers Tag pull requests created by maintainers text change PRs with text changes.
Projects
None yet
Milestone
2.18.0
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@jurajkusnier @LukasLechnerDev @SamuraiKek @CV113 @marcauberer