-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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.
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
[RNMobile] Simplify media insertion flow Part 2 - media upload #29547
[RNMobile] Simplify media insertion flow Part 2 - media upload #29547
Conversation
Size Change: +89 B (0%) Total Size: 1.4 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM too 😄
…nto rnmobile/simplify_image_insertion_flow-media-upload
…on (#29548) * Track the clientId of the block that is inserted. * implemented auto opening utilizing last block inserted from the store * added dismissal support for the auto opening picker to the UI tests. * Updated Dismiss button in closePicker function to Cancel
ee4d502
into
rnmobile/simplify_image_insertion_flow-redux-changes
👋 @jd-alexander ! I noticed that the PR description here says:
I also noticed that #29546 is still open. Wondering if that other PR should be closed or perhaps we should clarify/amend the description here? Not a big deal, I was just a bit confused. |
@mchowning yes, this comment needs removal. I will do that now. Sorry for the confusion.
Yes, so that PR is still open because even though all the child PRs have been merged into it base/parent PR, we ended up having to do a refactor to resolve some other issues that were discovered during the final review process. So now #30123 will be merged into #29546 once the cleanup there is complete. |
* created actions for adding and clearing last inserted block event. * added reducer for determining new state based on the action * added selector to query the state for the last block inserted * [RNMobile] Simplify media insertion flow Part 2 - media upload (#29547) * added autoOpenMediaUpload prop to the MediaPlaceholder * Added the auto-opening capabilities to the MediaUpload component. * Added documentation for the new autoOpenMediaUpload prop * renamed autoOpenMediaUpload to autoOpen in the MediaUpload component. * [RNMobile] Simplify media insertion flow - Part 3 component integration (#29548) * Track the clientId of the block that is inserted. * implemented auto opening utilizing last block inserted from the store * added dismissal support for the auto opening picker to the UI tests. * Updated Dismiss button in closePicker function to Cancel * Added release notes for auto-opening. * [RNMobile] Refactor simplify media flow redux store changes (#30123) * Moved the last block inserted actions from editor to the block-editor * Moved the last block inserted reducer from editor to the block-editor * Moved the last block inserted selector from editor to the block-editor * Fixed es-lint error. * Moved last block inserted actions test from editor to the block-editor * Moved last block inserted reducer test from editor to the block-editor * Moved last block inserted selector test from editor to the block-editor * Moved all calls to last block inserted from editor to block-editor * last block inserter usage in menu native migrated : editor to block-editor * [RNMobile] Refactor: Simplify media flow redux migration (#30238) * Add meta argument to insertBlock action * Add inserter menu source * Update last block inserted reducer Instead of using specific actions for tracking the last block inserted, it uses the actions related to the insertion flow. * Add get last block inserted selector * Refactor gallery edit component withSelect and withDispatch logic has been refactored to use useSelect and useDispatch hooks * Refactor image edit component wasBlockJustInserted is now calculated with getLastBlockInserted * Refactor video edit component wasBlockJustInserted is now calculated with getLastBlockInserted * Fix reset blocks action in last block inserted reducer * Add source param to wasBlockJustInserted selector * Simplify withSelect part of video block * Removed add/clear last block inserted actions and tests due to refactor * Removed addLastBlockInserted from the insert menu's onPress. * rewrote the tests for the lastBlockInserted reducer. * rewrote tests for wasBlockJustInserted selector. * optimized clientId * removed unneeded clientId. * put the expectedSource inside the test meta object. * used expectedState insted {} for state related tests. * used expectedState instead {} for state related tests. * removed parentheses from describe name. * return the same state instead of empty state. * made changes to test name so its intent is clearer. * made the insertion source optional. Co-authored-by: Carlos Garcia <fluiddot@gmail.com> * added wasBlockJustInserted prop needed after merge with trunk. * removed updateSelection from reducer so it's updated at all times. Co-authored-by: Carlos Garcia <fluiddot@gmail.com>
gutenberg-mobile
PR wordpress-mobile/gutenberg-mobile#2700Was merged into Part 1
Description
These changes introduce auto-opening capabilities to the
MediaUpload
component. Once theautoOpenMediaUpload
prop istrue
then the picker opens.How has this been tested?
To test this behavior, you can add
autoOpenMediaUpload={ true }
flag to theMediaPlaceholder
component of either theImage
,Video
orGallery
component. This will allow auto-opening capabilities on iOS and Android when the block is inserted or rendered.Screenshots
Image
Types of changes
autoOpenMediaUpload
prop in theMediaUpload
andMediaPlaceholder
components.delay
on iOS when opening the picker because it was observed that without it, the inserter's close behavior closes the picker while it's opening. I know we aren't fond of utilizingdelay
in our UI code, so the only other option would be to somehow pass the dismissal event of theInserter
bottom sheet down to theMediaUpload
component. I wasn't sure how to do that at a glance and I wasn't sure it was worth the effort since, I have seenTimeout
/delay
use cases across the codebase, so a second opinion on this before merge would be helpful.Checklist: