Skip to content
This repository has been archived by the owner on Oct 4, 2023. It is now read-only.

Fix mobile crash on drawer dismiss #3871

Merged
merged 3 commits into from
Aug 10, 2023
Merged

Fix mobile crash on drawer dismiss #3871

merged 3 commits into from
Aug 10, 2023

Conversation

dharit-tan
Copy link
Contributor

@dharit-tan dharit-tan commented Aug 10, 2023

Description

EDIT: found the real bug - we had ordered events incorrectly in the callback hooked into the drawer slideOut animation. We were actually setting visibility to false first, and then 'closing', this entire time!

Bug on mobile prod - if you swipe down to dismiss a drawer that reads drawer data like this: const { data } = useDrawer(drawerName), app crashes. For some reason even after dismissing, the app tries to render the drawer, but now the data field in drawer slice is null. Easy fix is to just add const { collectionId } = data ?? {} to avoid destructuring null, but i feel like there should be a better pattern here. /should maybe figure out why the dismissed drawer is still being rendered. Any ideas?
Can't add ?? {} in the selector or hook bc that gives type errors when we try to destructure in a component.
Example drawer is BlockMessagesDrawer (from profile screen top right)

Can bikeshed this later but probably good to get a fix out to prod asap.

Dragons

Is there anything the reviewer should be on the lookout for? Are there any dangerous changes?

How Has This Been Tested?

Local ios stage

How will this change be monitored?

For features that are critical or could fail silently please describe the monitoring/alerting being added.

Feature Flags

Are all new features properly feature flagged? Describe added feature flags.

@dharit-tan dharit-tan changed the title Fix drawer dismiss crash Fix mobile crash on drawer dismiss Aug 10, 2023
Copy link
Contributor

@dylanjeffers dylanjeffers left a comment

Choose a reason for hiding this comment

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

great job, can also do const collectionId = data?.collectionId for these, but all good either way :)

Copy link
Contributor

@rickyrombo rickyrombo left a comment

Choose a reason for hiding this comment

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

What a find!

IIUC: onFinished calls onClose, which would set the state back to closing but without any data?

@dharit-tan
Copy link
Contributor Author

dharit-tan commented Aug 10, 2023

I think onFinished calls onClose which sets to closing, and then onClosed (note closed instead of close) sets to false.
Monitoring the actions in redux, the ordering of events looks correct.

@rickyrombo
Copy link
Contributor

yeah makes sense, saying the same thing, I was describing the error case 👍

@dharit-tan dharit-tan merged commit 998d44b into main Aug 10, 2023
2 checks passed
@dharit-tan dharit-tan deleted the rt-drawer branch August 10, 2023 19:48
dharit-tan added a commit that referenced this pull request Aug 10, 2023
audius-infra pushed a commit that referenced this pull request Aug 12, 2023
[3436c20] [PAY-1701] Fix "Share to DMs" to work through InboxUnavailableModal (#3874) Marcus Pasell
[a740243] Add sdk:update-hotfix (#3875) Dylan Jeffers
[a25fd19] [C-2759] Make donation link external (#3872) Dylan Jeffers
[15f056c] [PAY-1630] Wire up purchase content sagas (#3834) Randy Schott
[998d44b] Fix mobile crash on drawer dismiss (#3871) Reed
[7d0e0b3] [PAY-1430] "Leaving Audius" Warning for External Links (and createModal helper) (#3860) Marcus Pasell
[bee8bd1] Remove .only on upload cypress test (#3869) Raymond Jacobson
[4c0b25f] [C-2926] Implement selected values for upload contextual menu fields (#3848) Dylan Jeffers
[5773578] Preserve CIDs for track and collection cover arts (#3866) Marcus Pasell
[be0d278] [C-2930] Fix extra space after username in tip to unlock modal (#3845) nicoback2
[f5320be] QA-588 Fix collection card profile link  (#3853) nicoback2
[360416e] Fix broken playlist fetch via resolve (#3863) Raymond Jacobson
[2dc2c29] [PAY-1695] DMs: Entrypoint Analytics (#3862) Marcus Pasell
[f80d366] Minor improvements to SEO flow merged in #3859 (#3861) Raymond Jacobson
[b99d62f] Add nodes to env for SEO support (#3859) Raymond Jacobson
[20476ee] [C-2941] Modify cloudflare worker to pull in SEO data from discovery nodes (#3858) Raymond Jacobson
[7f79830] [C-2879] Add validation to single track upload flow (#3855) Kyle Shanks
[6f4fc89] [C-2940] Update google analytics tags and fix embed build (#3856) Raymond Jacobson
[3469c89] [C-2852 PLAT-1094 PLAT-1093] Add fetch collection by permalink (#3751) Dylan Jeffers
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants