Skip to content

Commit d247402

Browse files
authored
fix crash when answering form question (ResearchKit#1599)
* fix crash when answering last question in a form, but there's subsequent informational (non-question) sections * Update ResearchKitUI/Common/Step/Form Step/ORKFormStepViewController.m * incorporate PR feedback * update docs
1 parent 5a96848 commit d247402

File tree

1 file changed

+60
-37
lines changed

1 file changed

+60
-37
lines changed

ResearchKitUI/Common/Step/Form Step/ORKFormStepViewController.m

Lines changed: 60 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1110,7 +1110,7 @@ - (void)setShouldPresentInReview:(BOOL)shouldPresentInReview {
11101110

11111111
#pragma mark Helpers
11121112

1113-
- (ORKFormStep *)formStep {
1113+
- (ORKFormStep *)formStep {
11141114
NSAssert(!self.step || [self.step isKindOfClass:[ORKFormStep class]], nil);
11151115
return (ORKFormStep *)self.step;
11161116
}
@@ -1384,53 +1384,76 @@ - (BOOL)didAutoScrollToNextItem:(ORKFormItemCell *)cell {
13841384
return YES;
13851385
}
13861386

1387-
- (BOOL)shouldAutoScrollToNextSection:(NSIndexPath *)indexPath {
1388-
if (![self _isAutoScrollEnabled]) {
1389-
return NO;
1387+
1388+
/// The proposed destination index path for auto-scrolling to the nexr section.
1389+
/// @returns The destination index path that should be used when scrolling to the next question, or `nil` if no auto-scroll should occur.
1390+
- (NSIndexPath *_Nullable)indexPathForAutoScrollingToNextSectionAfter:(NSIndexPath *)indexPath {
1391+
if (![self _isAutoScrollEnabled] || _autoScrollCancelled) {
1392+
return nil;
13901393
}
13911394

1392-
BOOL result = YES;
1393-
13941395
NSIndexPath *nextIndexPath = [NSIndexPath indexPathForRow:0 inSection:(indexPath.section + 1)];
13951396

1396-
// Technically, cellForRowAtIndexPath could return nil if the tableView hasn't decided to cache the cell
1397-
// Guarantee ourselves a cell by using dequeueReusableCellWithIdentifier—the only reason we need the cell is to test
1398-
// the cell's type, not for actual display, so using any cell paired with the reuseIdentifier should be fine
1399-
// do *not* use dequeueReusableCellWithIdentifier:indexPath: since that method should only be called within the dataSource
1400-
// tableView:cellForRowAtIndexPath: method
1401-
ORKFormItem *nextFormItem = [self _formItemForIndexPath:nextIndexPath];
14021397
ORKTableCellItemIdentifier *nextItemIdentifier = [_diffableDataSource itemIdentifierForIndexPath:nextIndexPath];
1403-
NSString *nextReuseIdentifier = [self cellReuseIdentifierFromFormItem:nextFormItem cellItemIdentifier:nextItemIdentifier];
1398+
if (nextItemIdentifier) {
1399+
// Technically, cellForRowAtIndexPath could return nil if the tableView hasn't decided to cache the cell
1400+
// Guarantee ourselves a cell by using dequeueReusableCellWithIdentifier—the only reason we need the cell is to test
1401+
// the cell's type, not for actual display, so using any cell paired with the reuseIdentifier should be fine
1402+
// do *not* use dequeueReusableCellWithIdentifier:indexPath: since that method should only be called within the dataSource
1403+
// tableView:cellForRowAtIndexPath: method
1404+
ORKFormItem *nextFormItem = [self _formItemForIndexPath:nextIndexPath];
1405+
if (!nextFormItem) {
1406+
return nil;
1407+
}
1408+
NSString *nextReuseIdentifier = [self cellReuseIdentifierFromFormItem:nextFormItem cellItemIdentifier:nextItemIdentifier];
1409+
// Can't autoscroll to something that doesn't exist
1410+
UITableViewCell *nextCell = [_tableView dequeueReusableCellWithIdentifier:nextReuseIdentifier];
1411+
if (!nextCell) {
1412+
return nil;
1413+
}
1414+
// Don't autoscroll to a cell that already has an answer.
1415+
if (self.savedAnswers[nextFormItem.identifier]) {
1416+
return nil;
1417+
}
1418+
return nextIndexPath;
1419+
} else if (@available(iOS 15, *)) {
1420+
NSString *nextSectionIdentifier = [_diffableDataSource sectionIdentifierForIndex:nextIndexPath.section];
1421+
ORKFormItem *formItem = [self _formItemForFormItemIdentifier:nextSectionIdentifier];
1422+
if (!formItem) {
1423+
return nil;
1424+
} else {
1425+
// We need to scroll to a zero-item section. This requires us to adjust the index path.
1426+
return [NSIndexPath indexPathForRow:NSNotFound inSection:nextIndexPath.section];
1427+
}
1428+
} else {
1429+
return nil;
1430+
}
1431+
}
14041432

1405-
result = result && (_autoScrollCancelled == NO);
14061433

1407-
// can't autoscroll to something that doesn't exist
1408-
UITableViewCell *nextCell = [_tableView dequeueReusableCellWithIdentifier:nextReuseIdentifier];
1409-
result = result && (nextCell != nil);
1410-
1411-
// don't autoscroll to a cell that already has an answer
1412-
result = result && (self.savedAnswers[nextFormItem.identifier] == nil);
1413-
1414-
return result;
1434+
/// Determines if we should auto-scroll to the next section
1435+
- (BOOL)shouldAutoScrollToNextSectionAfter:(NSIndexPath *)indexPath {
1436+
return [self indexPathForAutoScrollingToNextSectionAfter:indexPath] != nil;
14151437
}
14161438

1417-
- (void)autoScrollToNextSection:(NSIndexPath *)indexPath {
1439+
1440+
- (void)autoScrollToNextSectionAfter:(NSIndexPath *)indexPath {
14181441
if (![self _isAutoScrollEnabled]) {
14191442
return;
14201443
}
1421-
1422-
NSIndexPath *nextIndexPath = [NSIndexPath indexPathForRow:0 inSection:(indexPath.section + 1)];
1423-
UITableView *tableView = self.tableView;
1424-
UITableViewCell *nextCell = [tableView cellForRowAtIndexPath:nextIndexPath];
1425-
BOOL didChangeFocus = [self focusUnansweredCell:nextCell];
1426-
1427-
if (didChangeFocus == YES) {
1428-
// if we did change focus, then that will perform the scrolling
1429-
} else {
1430-
// if we didn't change focus, then we need to handle the scrolling here
1444+
NSIndexPath *scrollDestinationIndexPath = [self indexPathForAutoScrollingToNextSectionAfter:indexPath];
1445+
if (!scrollDestinationIndexPath) {
1446+
// If the index path returned by -shouldAutoScrollToNextSection: is nil, we're not supposed to auto-scroll to the next section.
1447+
return;
1448+
}
1449+
UITableViewCell *nextCell = [self.tableView cellForRowAtIndexPath:scrollDestinationIndexPath];
1450+
if (!(nextCell && [self focusUnansweredCell:nextCell])) {
1451+
// if we didn't change focus, we need to scroll.
1452+
// otherwise (if we did change focus), that'll take care of the scrolling for us.
14311453
dispatch_after(dispatch_time(DISPATCH_TIME_NOW, DelayBeforeAutoScroll * NSEC_PER_SEC), dispatch_get_main_queue(), ^{
1432-
[tableView scrollToRowAtIndexPath:nextIndexPath atScrollPosition:UITableViewScrollPositionTop animated:YES];
1454+
[_tableView scrollToRowAtIndexPath:scrollDestinationIndexPath atScrollPosition:UITableViewScrollPositionTop animated:YES];
14331455
});
1456+
// if we did change focus, then that will perform the scrolling
14341457
}
14351458
}
14361459

@@ -2020,8 +2043,8 @@ - (void)finishHandlingFormItemCellDidResignFirstResponder:(ORKTableCellItemIdent
20202043
NSString *sectionIdentifier = [[snapshot sectionIdentifiers] objectAtIndex:indexPath.section];
20212044
ORKFormItemCell *cell = ORKDynamicCast([self.tableView cellForRowAtIndexPath:indexPath], ORKFormItemCell);
20222045

2023-
if (cell.isLastItem && [self shouldAutoScrollToNextSection:indexPath]) {
2024-
[self autoScrollToNextSection:indexPath];
2046+
if (cell.isLastItem && [self shouldAutoScrollToNextSectionAfter:indexPath]) {
2047+
[self autoScrollToNextSectionAfter:indexPath];
20252048
return;
20262049
} else if (cell.isLastItem && indexPath.section == (snapshot.numberOfSections - 1) && ![_identifiersOfAnsweredSections containsObject:sectionIdentifier]) {
20272050
if (![self allNonOptionalFormItemsHaveAnswers]) {
@@ -2267,7 +2290,7 @@ - (BOOL)scrollNextSectionToVisibleFromIndexPath:(NSIndexPath *)indexPath {
22672290
if (allowAutoScrolling == YES) {
22682291
// only allow autoscrolling to the next section if the next section exists
22692292
if ((indexPath.section + 1) < [snapshot numberOfSections]) {
2270-
[self autoScrollToNextSection:indexPath];
2293+
[self autoScrollToNextSectionAfter:indexPath];
22712294
handledAutoScroll = YES;
22722295
} else {
22732296
// We would go to the next section, but we literally can't

0 commit comments

Comments
 (0)