Skip to content
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: Crash navigating to notification options - WPB-11062 #2021

Merged
merged 2 commits into from
Oct 15, 2024

Conversation

samwyndham
Copy link
Contributor

@samwyndham samwyndham commented Oct 11, 2024

Issue

When building using iOS 18 SDK on iOS 18 devices there is a crash when navigating to the notifications options screen. This is because we are dequeuing the footer from inside ConversationNotificationOptionsViewController.collectionView(_:layout:referenceSizeForFooterInSection:) which we should not be doing.

The fix is to create a separate sizing footer view that we only use for generating the reference size.

Testing

This must be tested using an Xcode 16 (iOS 18SDK) build on a iOS 18 device.

  1. Navigate to any group conversation
  2. Go to the details (by clicking the title)
  3. Tap Notifications
  4. There should be no crash and the footer is displayed correctly

Checklist

  • Title contains a reference JIRA issue number like [WPB-XXX].
  • Description is filled and free of optional paragraphs.
  • Adds/updates automated tests.

@echoes-hq echoes-hq bot added the echoes: maintenance Maintenance activity - Refactoring , Preventive , Improvements to code , Performance improvements label Oct 11, 2024
Copy link
Contributor

github-actions bot commented Oct 11, 2024

Test Results

    2 files    294 suites   3m 45s ⏱️
1 812 tests 1 812 ✅ 0 💤 0 ❌
1 820 runs  1 820 ✅ 0 💤 0 ❌

Results for commit 2afefbf.

♻️ This comment has been updated with latest results.

@samwyndham samwyndham requested review from agisilaos, a team, caldrian and johnxnguyen and removed request for a team and johnxnguyen October 11, 2024 13:49
Copy link
Contributor

@caldrian caldrian left a comment

Choose a reason for hiding this comment

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

nice work!

@samwyndham samwyndham added this pull request to the merge queue Oct 15, 2024
Merged via the queue into develop with commit 6c00b9d Oct 15, 2024
12 checks passed
@samwyndham samwyndham deleted the fix/WPB-11062-notification-options-crash branch October 15, 2024 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
echoes: maintenance Maintenance activity - Refactoring , Preventive , Improvements to code , Performance improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants