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.

Sign up for GitHub

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

Jump to bottom

fix: toast crash [WPB-4937] #2293

Merged
merged 7 commits into from
Oct 4, 2023
Merged

fix: toast crash [WPB-4937] #2293

merged 7 commits into from
Oct 4, 2023

Conversation

Garzas
Copy link
Contributor

@Garzas Garzas commented Oct 2, 2023 •
edited by github-actions bot
Loading

BugWPB-4937 [Android]- Crash on any screen that a snackbar/toast is shown

PR Submission Checklist for internal contributors

  • The PR Title

    • conforms to the style of semantic commits messages¹ supported in Wire's Github Workflow²
    • contains a reference JIRA issue number like SQPIT-764
    • answers the question: If merged, this PR will: ... ³

  • The PR Description

    • is free of optional paragraphs and you have filled the relevant parts to the best of your ability

What's new in this PR?

Issues

Swipeable composable caused crash for snackbar when user closes screen

Solutions

Implemented new swipeable snackbar to avoid crashes when closing screen using anchor draggable

@Garzas Garzas requested review from gongracr, ohassine and a team October 2, 2023 11:33
@Garzas Garzas self-assigned this Oct 2, 2023
@Garzas Garzas requested review from typfel, alexandreferris and borichellow and removed request for a team October 2, 2023 11:33
@github-actions
Copy link
Contributor

github-actions bot commented Oct 2, 2023 •
edited
Loading

Test Results

629 tests  ±0   629 ✔️ ±0   6m 52s ⏱️ +23s
  92 suites ±0       0 💤 ±0 
  92 files   ±0       0 ❌ ±0 

Results for commit 3371876. ± Comparison against base commit 7340e57.

♻️ This comment has been updated with latest results.

@AndroidBob
Copy link
Collaborator

Build 1097 failed.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 2, 2023

APKs built during tests are available here. Scroll down to Artifacts!

@AndroidBob
Copy link
Collaborator

Build 1098 succeeded.

The build produced the following APK's:

borichellow
borichellow approved these changes Oct 3, 2023
View reviewed changes
Copy link
Contributor

@borichellow borichellow left a comment

Choose a reason for hiding this comment

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

Looks good to me :)

app/src/main/kotlin/com/wire/android/WireApplication.kt Outdated
@@ -112,7 +112,7 @@ class WireApplication : Application(), Configuration.Provider {
.detectDiskReads()
.detectDiskWrites()
.penaltyLog()
.penaltyDeath()
// .penaltyDeath()
Copy link
Contributor

Choose a reason for hiding this comment

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

why is it here?

gongracr
gongracr approved these changes Oct 3, 2023
View reviewed changes
Copy link
Contributor

@gongracr gongracr left a comment

Choose a reason for hiding this comment

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

Nice and beautiful solution! 🏖️ Just added a suggestion to also adequate scaffold component on Conversation screen to use wire generic one. But again, feel free to ignore if it becomes too much of a workload and is out of scope of this PR, so I will not block it.

app/src/main/kotlin/com/wire/android/ui/home/conversations/ConversationScreen.kt Show resolved Hide resolved
@github-actions
Copy link
Contributor

github-actions bot commented Oct 4, 2023

APKs built during tests are available here. Scroll down to Artifacts!

@github-actions
Copy link
Contributor

github-actions bot commented Oct 4, 2023

APKs built during tests are available here. Scroll down to Artifacts!

@AndroidBob
Copy link
Collaborator

Build 1138 succeeded.

The build produced the following APK's:

@Garzas Garzas added this pull request to the merge queue Oct 4, 2023
@codecov
Copy link

codecov bot commented Oct 4, 2023

Codecov Report

Merging #2293 (3371876) into develop (cb0ad6b) will increase coverage by 0.64%.
Report is 2 commits behind head on develop.
The diff coverage is 56.36%.

@@              Coverage Diff              @@
##             develop    #2293      +/-   ##
=============================================
+ Coverage      40.72%   41.36%   +0.64%     
- Complexity      1034     1046      +12     
=============================================
  Files            323      324       +1     
  Lines          11748    11731      -17     
  Branches        1564     1554      -10     
=============================================
+ Hits            4784     4853      +69     
+ Misses          6510     6410     -100     
- Partials         454      468      +14
Files Coverage Δ
...rc/main/kotlin/com/wire/android/ui/WireActivity.kt 0.00% <ø> (ø)
...kotlin/com/wire/android/ui/home/HomeStateHolder.kt 0.00% <ø> (ø)
...id/ui/home/conversationslist/ConversationRouter.kt 0.00% <ø> (ø)
...oid/ui/home/conversationslist/model/SearchQuery.kt 100.00% <100.00%> (ø)
...d/ui/userprofile/avatarpicker/AvatarPickerState.kt 0.00% <ø> (ø)
...id/ui/sharing/ImportMediaAuthenticatedViewModel.kt 0.00% <0.00%> (ø)
...ome/conversationslist/ConversationListViewModel.kt 57.46% <53.06%> (+17.13%) ⬆️

... and 4 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7340e57...3371876. Read the comment docs.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 4, 2023

APKs built during tests are available here. Scroll down to Artifacts!

@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 4, 2023
@Garzas Garzas added this pull request to the merge queue Oct 4, 2023
@AndroidBob
Copy link
Collaborator

Build 1149 succeeded.

The build produced the following APK's:

Merged via the queue into develop with commit 3522b4a Oct 4, 2023
11 checks passed
@Garzas Garzas deleted the fix/snackbar-crash branch October 4, 2023 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@borichellow borichellow borichellow approved these changes

@gongracr gongracr gongracr approved these changes

@ohassine ohassine Awaiting requested review from ohassine

@typfel typfel Awaiting requested review from typfel typfel was automatically assigned from wireapp/android

@alexandreferris alexandreferris Awaiting requested review from alexandreferris alexandreferris was automatically assigned from wireapp/android

Assignees

@Garzas Garzas

Labels
size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Garzas @AndroidBob @gongracr @borichellow @yamilmedina