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

Add home screen risk cell update button countdown #866

Merged
merged 10 commits into from Jul 9, 2020

Conversation

johannesrohwer
Copy link
Member

Description

This PR implements a countdown for the update button that is shown for the risk cell on the home screen. (see JIRA 1626)

Checklist

  • This PR does not change text that hasn't been signed off with the RKI. Have a look at the pinned issue 440
  • Test your changes as thoroughly as possible before you commit them. Preferably, automate your test by unit/integration tests.
  • Set a speaking title. Format: {task_name} (closes #{issue_number}). For example: Use logger (closes # 41)
  • Make sure that your PR is not introducing unnecessary reformatting (e.g., introduced by on-save hooks in your IDE)

@johannesrohwer johannesrohwer added the enhancement Improvement of an existing feature label Jul 7, 2020
@johannesrohwer johannesrohwer requested a review from a team July 7, 2020 16:24
* develop:
  Bump version + remove turkish
  Fixed wrong implementation of shouldPerformExposureDetection
  Removed custom html color definitions.
  Applied default color to attributed string. Only updated  view when html parsing was successful.
  Applied default color.
  Fixed default font.
  chore: merge Risk & ExposureSubmission errors, remove duplicates as much as possible
  Risk Calculation Error Message Refinement
  Refactored ActivateCollectionViewCell configurator architecture.
  Removed HTML reload as it is not required anymore with current implementation thereby addressing JIRA ticket 1500.
  Inferred animation duration from loaded frames.
  Applied animated tracing status in configurator.
  Removed superfluous source line.
  Added animated icons for enabled tracing status.
src/xcode/ENA/ENA/Source/Scenes/Home/HomeInteractor.swift Outdated Show resolved Hide resolved
let nextUpdate = riskProvider.nextExposureDetectionDate()
countdownTimer = CountdownTimer(countdownTo: nextUpdate)
countdownTimer?.delegate = self
countdownTimer?.start()
Copy link
Member

Choose a reason for hiding this comment

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

I see, the timer invalidate is only called in the CoutdownTimer class:

 deinit {
		timer?.invalidate()
		timer = nil
	} 

My question is, if the app goes background, the Timer still runs? What will happened if the user opens the App after 2 days?

Copy link
Member Author

Choose a reason for hiding this comment

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

thanks for bringing this up, I discovered a bug in my implementation where the timer was not stopped as I expected due to a reference cycle. I added a manual invalidate() method to make sure the timer is handled correctly.
Regarding the opening of the app after some time has passed: The timer is guaranteed to be up-to-date since the scene delegate will trigger the updateExposureState() method of the home VC when the app comes into foreground.
To make sure the timer does not drain the users battery when the app goes into background, I added a few lines that invalidate the handler when the app goes into background.

Copy link
Member

@melloskitten melloskitten left a comment

Choose a reason for hiding this comment

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

I have added some comments and cleaned up some of the code. Would be cool if you looked through these commits, and checked whether they make sense to you. Thanks! 👍

In case there aren't enough tests: Please make sure to add tests, cheers! 😄

Copy link
Member

@haosap haosap left a comment

Choose a reason for hiding this comment

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

Well done!💯

@haosap haosap merged commit 0b98216 into develop Jul 9, 2020
@haosap haosap deleted the feature/home-screen-update-timer branch July 9, 2020 11:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Improvement of an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants