Skip to content

[FIX] Black screen during Share#3320

Merged
diegolmello merged 6 commits intoRocketChat:developfrom
cprice-kgi:share-black-screen
Sep 14, 2021
Merged

[FIX] Black screen during Share#3320
diegolmello merged 6 commits intoRocketChat:developfrom
cprice-kgi:share-black-screen

Conversation

@cprice-kgi
Copy link
Copy Markdown
Contributor

Proposed changes

Something about RNSplashScreen.hide is causing an issue here. I wrapped the call in a try/catch.

Issue(s)

#3088

How to test or reproduce

Try to share an image either from within the app or from an another application to the app. After investigation, I have found this issue appears only when the server has the mobile screen lock value enabled. If it is disabled, sharing will work fine. If enabled, the call to RNSplashScreen.hide will cause a black screen to appear, and the user is unable to proceed.

Screenshots

Types of changes

  • [ x] Bugfix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change which improves a current function)
  • New feature (non-breaking change which adds functionality)
  • Documentation update (if none of the other choices apply)

Checklist

  • [x ] I have read the CONTRIBUTING doc
  • [ x] I have signed the CLA
  • [ x] Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works (if applicable)
  • I have added necessary documentation (if applicable)
  • Any dependent changes have been merged and published in downstream modules

Further comments

N/A

Copy link
Copy Markdown
Contributor

@LevyCosta LevyCosta left a comment

Choose a reason for hiding this comment

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

lgtm

const {
title_link, image_url, image_type, video_url, video_type
} = attachment;
const { title_link, image_url, image_type, video_url, video_type } = attachment;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Unrelated, but prettier asked me to change.
It won't hurt :)

@diegolmello
Copy link
Copy Markdown
Member

The issue is that we're calling hide without having a splash screen on share extension.
That triggers react-native-bootsplash has not been initialized, similar to zoontek/react-native-bootsplash#264
Recently they wrapped the hide call in a try/catch in the example zoontek/react-native-bootsplash@41e32e8#diff-f92b617bbc90bafc01461177b82175f320578d91293fddfbc810bacb339898edR20

@diegolmello diegolmello merged commit 28d7a1b into RocketChat:develop Sep 14, 2021
@diegolmello
Copy link
Copy Markdown
Member

@cprice-kgi Merged. Thanks for your contribution :)

@jsalatiel
Copy link
Copy Markdown

Is this the same problem that happens when one tries to share a audio received on whatsapp with someone on rockechat ? After choosing the contact for share, rocketchat gets stuck on a blank window. ( on android , running version 4.20)

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.21%. Comparing base (b795d4b) to head (3b08b43).
⚠️ Report is 1736 commits behind head on develop.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #3320   +/-   ##
========================================
  Coverage    71.21%   71.21%           
========================================
  Files          153      153           
  Lines         2758     2758           
  Branches       648      648           
========================================
  Hits          1964     1964           
  Misses         760      760           
  Partials        34       34           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants