Preserve tableView selection when app entering background #1594
Conversation
There was a problem hiding this comment.
Hey there @charliescheer !! Changes look good, buuut sending you a couple annoying comments 😆
LMK what you think!!
Simplenote/SPAppDelegate.m
Outdated
| @@ -41,6 +41,7 @@ | |||
| @interface SPAppDelegate () | |||
|
|
|||
| @property (weak, nonatomic) SPModalActivityIndicator *signOutActivityIndicator; | |||
| @property (nonatomic) NSArray *selectedNotesEnteringBackground; | |||
There was a problem hiding this comment.
@charliescheer do you think we could encapsulate this in the NoteList itself?
(So that the AppDelegate doesn't keep track of IndexPath(s))
There was a problem hiding this comment.
Ooff that would have been so much easier if the whole thing was in swift. Yes I have made that change. 👍
Simplenote/SPAppDelegate.m
Outdated
| @@ -189,12 +190,19 @@ - (void)applicationDidEnterBackground:(UIApplication *)application | |||
| [self cleanupScrollPositionCache]; | |||
| [self syncWidgetDefaults]; | |||
| [self resetWidgetTimelines]; | |||
|
|
|||
| self.selectedNotesEnteringBackground = self.noteListViewController.tableView.indexPathsForSelectedRows; | |||
There was a problem hiding this comment.
Same note here, perhaps we could move this into the Note List itself?
(The appDelegate could relay the event, or the Note List could listen to the didEnterBackground notification)
| @@ -520,8 +529,6 @@ - (void)update | |||
| [self refreshTitle]; | |||
| [self refreshSearchBar]; | |||
|
|
|||
| BOOL isTrashOnScreen = self.isDeletedFilterActive; | |||
There was a problem hiding this comment.
This was a warning in the project that I probably caused recently, but I was tired of it staring at me. So tossing that in here to shut it up.
| @@ -29,7 +29,7 @@ extension URL { | |||
| } | |||
|
|
|||
| static func newNoteWidgetURL() -> URL { | |||
| guard var components = URLComponents.simplenoteURLComponents(with: Constants.newNotePath) else { | |||
| guard let components = URLComponents.simplenoteURLComponents(with: Constants.newNotePath) else { | |||
There was a problem hiding this comment.
another warning that was poking me in the brain.
| @@ -192,6 +194,9 @@ - (void)startListeningToNotifications { | |||
|
|
|||
| // Themes | |||
| [nc addObserver:self selector:@selector(themeDidChange) name:SPSimplenoteThemeChangedNotification object:nil]; | |||
|
|
|||
| // App Background | |||
| [nc addObserver:self selector:@selector(appWillEnterBackground) name:UIApplicationDidEnterBackgroundNotification object:nil]; | |||
There was a problem hiding this comment.
Nit: UIApplicationWillEnterForegroundNotification and you can remove the AppDelegate from the equation!
There was a problem hiding this comment.
yeah, good call. Done
Fix
Currently if you have notes selected in the notes table view and you put Simplenote into the background, the selection is lost. Not an ideal situation, I would expect that you could background and then return to Simplenote and the state of the app should be the same, so the selection would be preserved. So in this PR I have fixed that.
Test
Review
(Required) Add instructions for reviewers. For example:
Release
(Required) Add a concise statement to
RELEASE-NOTES.txtif the changes should be included in release notes. Include details about updating the notes in this section. For example: