feat: Insert from URL leverages a bottom sheet #54096
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dcalhoun
added
[Type] Enhancement
dcalhoun
added
the
Mobile App - Automation
|
Size Change: 0 B Total Size: 1.51 MB ℹ️ View Unchanged
|
|
Flaky tests detected in a46455c497737b69f02aabb1130534d2e74e75ab. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6043873311
|
|
👋🏻 @osullivanchris @paulforgione would either you be willing to review this solution for wordpress-mobile/gutenberg-mobile#4936 from a design perspective? You can find prototype builds for testing referenced in this PRs description. I am open to alternative UI or flows, but the bottom sheet approach felt the most appropriate for the UI patterns we have in place currently. You can review my rationale for this approach in wordpress-mobile/gutenberg-mobile#4936 (comment). Thanks! |
|
Thanks for the ping @dcalhoun . This looks much better. Good for me 👍 . One small thing. When I tap Insert from URL, the keyboard rises with the block insertor initially, before this bottom sheet animates up. Looks a little bit glitchy. But I believe this might be a general issue we have with keyboards/toolbars. https://github.com/WordPress/gutenberg/assets/50576199/4e5b35cc-970d-44a2-942a-6bf9957a71b0 |
dcalhoun
force-pushed
the
feat/insert-from-url-bottom-sheet
branch
from
a46455c
to
f45e95b
Compare
Thanks for the quick feedback. Super helpful! 🙇🏻
Yes, I observe similar animation oddities with this UI and elsewhere. I consider this to be in the scope of #37559, which hopefully would be addressed if we work on #42192. |
derekblank
approved these changes
Sep 4, 2023
This allows the MediaUploader component file to export only React components, which enables hot module reloading.
The usage of iOS `UIAlertController` via `react-native-prompt-android` resulted in an obscured alert on small devices in landscape orientation as there was not enough vertical space between the keyboard and the top of the editor view. Replacing the usage of alerts with the existing bottom sheet avoids the obscured UI.
This package is no longer installed or used. These referenced caused build errors.
This felt appropriate given this input is displayed for a user immediately after tapping "Insert from URL." Thus, the user likely expects a focused input and presented keyboard.
Align with content layout with that found in other existing bottom sheets.
Used for tests in the gutenberg-mobile repository.
dcalhoun
force-pushed
the
feat/insert-from-url-bottom-sheet
branch
from
50507e3
to
db66c34
Compare
This was referenced Sep 12, 2023
4 tasks
This was referenced Sep 27, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related PRs
What?
Replace the usage of an OS alert with a bottom sheet for the "Insert from URL"
flow for supported media blocks.
Why?
Fixes wordpress-mobile/gutenberg-mobile#4936.
The usage of iOS
UIAlertControllerviareact-native-prompt-androidresultedin an obscured alert on small devices in landscape orientation as there was not
enough vertical space between the keyboard and the top of the editor view.
Replacing the usage of alerts with the existing bottom sheet avoids the obscured
UI.
How?
Refactor MediaUploader to export constants seprately, to enable fast refresh
support through only exporting a React component.
Replace usage of
react-native-prompt-androidwith a bottom sheet containing atext input.
Given the refactor, stepping through the commits individually may simplify code review.
Testing Instructions
Valid Media
Invalid Media
Replace Media
Testing Instructions for Keyboard
n/a
Screenshots or screencast