-
Notifications
You must be signed in to change notification settings - Fork 6
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] #476 - 습관 인증 스톱워치 백그라운드에서 멈추는 문제 해결 #477
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.
👍 ⏲️
func sceneDidEnterBackground(_ scene: UIScene) { | ||
NotificationCenter.default.post(name: NSNotification.Name(Const.UserDefaultsKey.sceneDidEnterBackground), object: nil) | ||
UserDefaults.standard.setValue(Date(), forKey: Const.UserDefaultsKey.sceneDidEnterBackground) |
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.
와아..이렇게 할수 있군여 굳아이디어...!
// Background -> Foreground | ||
NotificationCenter.default.addObserver(self, selector: #selector(checkBackgroundTimer), name: NSNotification.Name(Const.UserDefaultsKey.sceneWillEnterForeground), object: nil) | ||
// Foreground -> Background | ||
NotificationCenter.default.addObserver(self, selector: #selector(checkBackgroundTimer(_:)), name: NSNotification.Name(Const.UserDefaultsKey.sceneDidEnterBackground), object: nil) |
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.
요기가 살짝 달라요!
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.
아하 굳이 필요없는 칭구 지워서 통일 시키겠슴다
NotificationCenter.default.addObserver(self, selector: #selector(checkBackgroundTimer), name: UIApplication.willResignActiveNotification, object: nil) | ||
NotificationCenter.default.addObserver(self, selector: #selector(checkBackgroundTimer), name: UIApplication.willEnterForegroundNotification, object: nil) |
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.
모든 옵저버에 대해 removeObserver를 해주는 부분이 필요할 것 같아요! 혹시 제가 놓쳤을까요?
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.
iOS 9 이후로 모든 옵저버에 대해서 항상 삭제해줄 의무는 없지만 아시다시피 다른 뷰컨트롤러에서 같은 이름의 notification 을 observe 하게되면 문제가 생기게 되요!
스톱워치의 경우 observer 가 있지 않은 상태에서 post 를 하면 의미가 없고(스톱워치가 아닌 다른 뷰에서 백그라운드->포그라운드 작업을 통해서 post 발생) 스톱워치 뷰컨이 등장하고(observer 등록), 백그라운드로 갔다가 다시 포그라운드로 돌아오는과정(post)이서 문제가 될 것 같지는 않아요! 하지만 remove 하면 메모리 관리적으로도 좋으니까 viewWillDisappear 에서 수동으로 호출해주면 좋을 것 같아요!
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.
이부분도 setNotification()
메서드에 넣으면 좋지 않을까요?
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.
네넵 근데 보니까 일단 저 노티에 해당하는 옵저버는 필요가 없는 친구였네요..
제가 scenedelegate에서 따로 실행할 부분을 넣고 그에 해당하는 옵저버는 지그 setNotification에 추가되어 있는 상태라 이 친구는 지우도록 하겠습니다
그리구 remove도 추가해ㅏㄹ게여!
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.
뷰내에서 해당 화면을 오랫동안 유지하지 않으면 정확한 값을 얻을 수 없다 등의 문구를 삽입해줘야할거 같아요 정리해서 슬랙으로 전달드려야할 듯 하네여.. 후우.. 저도 좀 더 찾아볼게요!
/// Foreground에 들어올 때 | ||
/// sceneDidEnterBackground라는 키워드로 저장해둔 값을 sceneWillEnterForeground라는 키워드 노티의 userInfo로 전달 | ||
func sceneWillEnterForeground(_ scene: UIScene) { | ||
guard let start = UserDefaults.standard.object(forKey: Const.UserDefaultsKey.sceneDidEnterBackground) as? Date else { return } | ||
let interval = Int(Date().timeIntervalSince(start)) | ||
NotificationCenter.default.post(name: NSNotification.Name(Const.UserDefaultsKey.sceneWillEnterForeground), object: nil, userInfo: ["time": interval]) |
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.
sceneWillEntrForeground
라는 메서드를 다른 곳에서 호출하지 않으니 퀵헬프 주석인 /// 보다는 역할을 설명하는 주석으로써 // 을 쓰는 것이 좀 더 어울려보입니당!
NotificationCenter.default.addObserver(self, selector: #selector(checkBackgroundTimer), name: UIApplication.willResignActiveNotification, object: nil) | ||
NotificationCenter.default.addObserver(self, selector: #selector(checkBackgroundTimer), name: UIApplication.willEnterForegroundNotification, object: nil) |
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.
iOS 9 이후로 모든 옵저버에 대해서 항상 삭제해줄 의무는 없지만 아시다시피 다른 뷰컨트롤러에서 같은 이름의 notification 을 observe 하게되면 문제가 생기게 되요!
스톱워치의 경우 observer 가 있지 않은 상태에서 post 를 하면 의미가 없고(스톱워치가 아닌 다른 뷰에서 백그라운드->포그라운드 작업을 통해서 post 발생) 스톱워치 뷰컨이 등장하고(observer 등록), 백그라운드로 갔다가 다시 포그라운드로 돌아오는과정(post)이서 문제가 될 것 같지는 않아요! 하지만 remove 하면 메모리 관리적으로도 좋으니까 viewWillDisappear 에서 수동으로 호출해주면 좋을 것 같아요!
NotificationCenter.default.addObserver(self, selector: #selector(checkBackgroundTimer), name: UIApplication.willResignActiveNotification, object: nil) | ||
NotificationCenter.default.addObserver(self, selector: #selector(checkBackgroundTimer), name: UIApplication.willEnterForegroundNotification, object: nil) |
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.
이부분도 setNotification()
메서드에 넣으면 좋지 않을까요?
@objc | ||
func checkBackgroundTimer(_ notification: NSNotification) { | ||
if let isValid = timer?.isValid { | ||
if isTimerOn && isValid { | ||
let time = notification.userInfo?["time"] as? Int ?? 0 | ||
currentTimeCount += time | ||
} | ||
} |
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.
포그라운드에서 백그라운드로 갈때는 notification 에 time 이 없어서 if let 같은 구문보다 위와 같이 옵셔널을 바인딩한 부분 좋은 거 같아요
그런데 애초에 포그라운드에서 백그라운드로 갈때 UserDefaults 를 쓰는것 의미가 있는데 notification 을 보내는 건 의미가 있을까 싶어요! 다른 역할이 있을까요?
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.
아 저 이것저것 해볼때 포그라운드에서 백그라운드로 갈 때 타이머를 멈춰주는 함수를 넣느라 그때 썼던건데
지금은 안사용하고 있어서 지우는게 맞겠네요.. 감사합니다..
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.
굳굳 덕분에 좋은 노티 알고가요~
@@ -99,7 +103,7 @@ class AuthTimerVC: UIViewController { | |||
// Background -> Foreground | |||
NotificationCenter.default.addObserver(self, selector: #selector(checkBackgroundTimer), name: NSNotification.Name(Const.UserDefaultsKey.sceneWillEnterForeground), object: nil) | |||
// Foreground -> Background | |||
NotificationCenter.default.addObserver(self, selector: #selector(checkBackgroundTimer(_:)), name: NSNotification.Name(Const.UserDefaultsKey.sceneDidEnterBackground), object: nil) | |||
NotificationCenter.default.addObserver(self, selector: #selector(checkBackgroundTimer), name: NSNotification.Name(Const.UserDefaultsKey.sceneDidEnterBackground), object: nil) |
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.
이것도 지워주시는걸 놓치신 듯 해요!
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.
아고 놓쳤네요.. 감사합니다
Merge branch 'develop' of https://github.com/TeamSparker/Spark-iOS into feature/TeamSparker#476
🔥Pull requests
⛳️ 작업한 브랜치
👷 작업한 내용
🚨참고 사항
일단은 백그라운드에 있는 경우에도 스톱워치가 작동하도록 하기 위해
background로 들어갈 때, foreground로 들어올때의 시간차를 초 단위로 구해
기존의 currentCountTime이라는 초 단위의 친구에게 더해주도록 했습니다.
백그라운드에 뒀다가 앱 자체가 갱신돼서 스플래시부터 시작하는 경우는 따로 이슈를 만들어서 고민해봐야 할 것 같습니다.
📸 스크린샷
📟 관련 이슈