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

Fixing crash when tapping timeline verification item #5558

Merged
merged 1 commit into from
Mar 17, 2022

Conversation

ouchadam
Copy link
Contributor

Type of change

  • Feature
  • Bugfix
  • Technical
  • Other :

Content

Fixes the wrong setArguments being called when triggering the verification bottom sheet

Motivation and context

Fixes a crash when tapping the item area instead of the buttons #5540

Screenshots / GIFs

BEFORE AFTER
crash after-verify

Tests

  • From web, verify another user
  • On mobile as the other user, tap the area surrounding the verification buttons

Tested devices

  • Physical
  • Emulator
  • OS version(s): 31 Sv2

- caused by the setArguments being called on the TimelineFragment not the bottomsheet we've just created
@ouchadam ouchadam requested review from a team and ericdecanini and removed request for a team March 16, 2022 16:20
@ouchadam ouchadam added the PR-Small PR with less than 20 updated lines label Mar 16, 2022
@@ -1771,13 +1771,11 @@ class TimelineFragment @Inject constructor(
}
is RoomDetailAction.ResumeVerification -> {
val otherUserId = data.otherUserId ?: return
VerificationBottomSheet().apply {
setArguments(VerificationBottomSheet.VerificationArgs(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this setArguments is actually TimelineFragment.setArguments

Copy link
Member

Choose a reason for hiding this comment

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

Ah, a good example of why I do not like to use apply CC @mnaturel !

@github-actions
Copy link

Unit Test Results

102 files  ±0  102 suites  ±0   1m 8s ⏱️ -11s
182 tests ±0  182 ✔️ ±0  0 💤 ±0  0 ±0 
598 runs  ±0  598 ✔️ ±0  0 💤 ±0  0 ±0 

Results for commit 4154f03. ± Comparison against base commit d1a77d2.

Copy link
Contributor

@ericdecanini ericdecanini left a comment

Choose a reason for hiding this comment

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

Simple fix 👍

@ouchadam ouchadam merged commit 36564d3 into develop Mar 17, 2022
@ouchadam ouchadam deleted the feature/adm/crash-when-tapping-verification-item branch March 17, 2022 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR-Small PR with less than 20 updated lines Z-Crash
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants