Page MenuHomePhabricator
Differential D6494

[native] Allow MediaGallery items to mount pre-selected
ClosedPublic
Actions

Authored by bartek on Feb 1 2023, 8:46 AM.
Tags
None
Referenced Files
F3245797: D6494.id21980.diff
Thu, Nov 14, 8:20 PM
Unknown Object (File)
Wed, Oct 30, 5:59 AM
Unknown Object (File)
Wed, Oct 30, 5:59 AM
Unknown Object (File)
Wed, Oct 30, 5:58 AM
Unknown Object (File)
Wed, Oct 30, 5:53 AM
Unknown Object (File)
Sep 30 2024, 3:21 AM
Unknown Object (File)
Sep 30 2024, 3:21 AM
Unknown Object (File)
Sep 30 2024, 3:21 AM
View All 63 Files
Subscribers
atul
tomek

Details

Reviewers
ashoat
• przemek
inka
ginsu
Commits
rCOMM382baa4e984b: [native] Allow MediaGallery items to mount pre-selected
Summary

For the approach described in https://linear.app/comm/issue/ENG-2411#comment-c4d6a4a4 to work, items can appear in MediaGalleryKeyboard preselected.
I need that MediaGalleryMedia component can be mounted with the pre-selected state already.

I basically copied code that animates the green checkmark from componentDidUpdate to componentDidMount.

Test Plan

Ensured that items that have isQueued = true when mounted have the green checkmark displayed and animated.

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

bartek created this revision.Feb 1 2023, 8:46 AM
bartek held this revision as a draft.
Herald added subscribers: atul, tomek. · View Herald TranscriptFeb 1 2023, 8:46 AM
bartek added a child revision: D6495: [native] Add function to open native picker.Feb 1 2023, 8:54 AM
Harbormaster completed remote builds in B15939: Diff 21762.Feb 1 2023, 9:03 AM
bartek published this revision for review.Feb 1 2023, 10:15 AM
ashoat requested changes to this revision.Feb 1 2023, 11:03 AM
ashoat added inline comments.
native/media/media-gallery-media.react.js
56

Instead of animating these on mount, I think we should initialize them to the correct value. We can move the initialization of focusProgress and checkProgress to constructor, and pick the values based on isActive and isQueued. What do you think?

This revision now requires changes to proceed.Feb 1 2023, 11:03 AM
bartek added inline comments.Feb 1 2023, 11:17 AM
native/media/media-gallery-media.react.js
56

The reason I animate them instead of just setting the value is to achieve animated effect visible on this video in ~0:08

bartek attached a referenced file: F355781: simulator-screen-recording-iphone-14-pro-2023-02-01-at-190533_sbSuHNEi.mp4. (Show Details)Feb 1 2023, 11:17 AM
ashoat accepted this revision.Feb 1 2023, 1:04 PM

Ah, that makes sense... without the animation, the user might not realize that the media listed at the start is not the same as it was before

This revision is now accepted and ready to land.Feb 1 2023, 1:04 PM
Closed by commit rCOMM382baa4e984b: [native] Allow MediaGallery items to mount pre-selected (authored by bartek). · Explain WhyFeb 4 2023, 10:03 AM
This revision was automatically updated to reflect the committed changes.
bartek added a commit: rCOMM382baa4e984b: [native] Allow MediaGallery items to mount pre-selected.

Revision Contents
Changeset List

PathSize
native/
media/
22 lines

Diff 21762

View Options

native/media/media-gallery-media.react.js

Loading...