Page MenuHomePhabricator
Differential D6730

[native] Add SafeAreaView to CameraModal, ImageModal and VideoPlaybackModal
ClosedPublic
Actions

Authored by • kuba on Feb 14 2023, 12:53 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Oct 28, 9:51 PM
Unknown Object (File)
Mon, Oct 28, 9:51 PM
Unknown Object (File)
Mon, Oct 28, 9:51 PM
Unknown Object (File)
Mon, Oct 28, 9:51 PM
Unknown Object (File)
Mon, Oct 28, 9:51 PM
Unknown Object (File)
Mon, Oct 28, 9:51 PM
Unknown Object (File)
Mon, Oct 28, 9:49 PM
Unknown Object (File)
Oct 13 2024, 9:24 PM
View All 91 Files
Subscribers
ashoat
atul

Details

Reviewers
tomek
inka
michal
Commits
rCOMMf592063bc747: [native] Add SafeAreaView to CameraModal, ImageModal and VideoPlaybackModal
Summary

Issue: ENG-2476

Android: In some modals app buttons are rendered outside SaveArea and covered by the system GUI, especially in landscape mode.

Images:

Screenshot_20230214-111147.png (1×2 px, 2 MB)

Screenshot_20230214-111200.png (1×2 px, 972 KB)

Test Plan

Tested new view on Android and iOS modals:

  • CameraModal (including staging view),
  • VideoPlaybackModal,
  • ImageModal.

Images:

Screenshot_20230214-110640.png (1×2 px, 1 MB)

Screenshot_20230214-110632.png (1×2 px, 1 MB)

Screenshot_20230214-110710.png (1×2 px, 990 KB)

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

• kuba created this revision.Feb 14 2023, 12:53 AM
Herald added subscribers: atul, ashoat. · View Herald TranscriptFeb 14 2023, 12:53 AM
• kuba edited the summary of this revision. (Show Details)Feb 14 2023, 12:58 AM
• kuba edited the test plan for this revision. (Show Details)
• kuba added reviewers: inka, michal.
Harbormaster returned this revision to the author for changes because remote builds failed.Feb 14 2023, 1:07 AM
Harbormaster failed remote builds in B16473: Diff 22520!
• kuba edited the summary of this revision. (Show Details)Feb 14 2023, 2:18 AM
• kuba edited the test plan for this revision. (Show Details)
• kuba updated this revision to Diff 22552.Feb 14 2023, 2:56 AM
• kuba edited the test plan for this revision. (Show Details)

Fixed imports

Harbormaster completed remote builds in B16505: Diff 22552.Feb 14 2023, 4:15 AM
• kuba requested review of this revision.Feb 14 2023, 4:15 AM
inka added inline comments.Feb 14 2023, 4:21 AM
native/media/camera-modal.react.js
619 ↗(On Diff #22552)

Is this View necessary? Since it has the same style as the parent, that is just flex: 1, it seems like it doesn't do much? I tried without it (on android), and it seemed to work

• kuba marked an inline comment as done.Feb 15 2023, 2:38 AM
• kuba added inline comments.
native/media/camera-modal.react.js
619 ↗(On Diff #22552)

I believe that it is. If you remove this View and rotate the phone to landscape mode you end up with X going outside the SaveAreaView.
I added styles to see it clearly:

  • SafeAreaView: backgroundColor: '#FF000040' (red)
  • View: backgroundColor: '#00FF0040' (green)

Attaching screens:
Without inner View:

Screenshot_20230215-113106.png (1×2 px, 1 MB)

With inner View:

Screenshot_20230215-113116.png (1×2 px, 1 MB)

• kuba attached a referenced file: F381658: Screenshot_20230215-113116.png. (Show Details)Feb 15 2023, 2:38 AM
• kuba attached a referenced file: F381657: Screenshot_20230215-113106.png. (Show Details)
tomek accepted this revision.Feb 16 2023, 2:09 AM
tomek added inline comments.
native/media/camera-modal.react.js
619 ↗(On Diff #22552)

It's surprising that we need it. I guess it might be related to the fact that the buttons are absolutely positioned.

647 ↗(On Diff #22552)

Do we have to provide both flex: 1 and stagingViewOverlay styles? Is flex: 1 really useful? It works in one direction, so the behavior might be different when a phone is rotated.

This revision is now accepted and ready to land.Feb 16 2023, 2:09 AM
inka added inline comments.Feb 16 2023, 2:23 AM
native/media/camera-modal.react.js
619 ↗(On Diff #22552)

Okay, thank you for the screenshots.

• kuba updated this revision to Diff 22646.Feb 16 2023, 3:09 AM
• kuba marked 2 inline comments as done.

Removed flex: 1 from staging view overlay

• kuba marked 2 inline comments as done.Feb 16 2023, 3:11 AM
• kuba added inline comments.
native/media/camera-modal.react.js
647 ↗(On Diff #22552)

I checked it and you are right. Everything works correctly without flex: 1 style. I removed it and updated the diff.

• kuba updated this revision to Diff 22647.Feb 16 2023, 3:23 AM

Removed flex: 1 from camera-modal

Harbormaster completed remote builds in B16550: Diff 22646.Feb 16 2023, 3:24 AM
Harbormaster completed remote builds in B16551: Diff 22647.Feb 16 2023, 3:38 AM
Closed by commit rCOMMf592063bc747: [native] Add SafeAreaView to CameraModal, ImageModal and VideoPlaybackModal (authored by kosmydel <kukubaczek@gmail.com>). · Explain WhyFeb 17 2023, 12:52 AM
This revision was automatically updated to reflect the committed changes.
kosmydel <kukubaczek@gmail.com> added a commit: rCOMMf592063bc747: [native] Add SafeAreaView to CameraModal, ImageModal and VideoPlaybackModal.

Revision Contents
Changeset List

Diff 22663

View Options

native/media/camera-modal.react.js

Loading...
View Options

native/media/image-modal.react.js

Loading...
View Options

native/media/video-playback-modal.react.js

Loading...