Skip to content
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

Add/mobile fullscreen image preview #17109

Conversation

cameronvoell
Copy link
Member

@cameronvoell cameronvoell commented Aug 20, 2019

Description

This PR covers fullscreen image preview in the gutenberg mobile editor as specified in gb-mobile issue 1286

WP iOS PR for testing: wordpress-mobile/WordPress-iOS#12834
fullscreenimage-sketch
Remaining tasks:

  1. Fade animation to fullscreen image preview from Image block editor
  2. Image Preview has correct aspect ratio in portrait and landscape
  3. Highlight border around image when image block is selected (same as video block)

Decided to commit behind a __DEV__ flag until rotation issues are resolved. Tasks below will be in a future PR.
4. Address dependency on Fix for Android modal rotation crash (wp-mobile PR 10383)
5. (•••) Button over Image
6. (•••) Button leads to BottomSheet with Edit and ViewFullscreen options

How has this been tested?

Tested as a submodule to gutenberg-mobile repo at latest develop (cf27fba )

Test steps:

  1. Open article in the block editor.
  2. Navigate to an image block with an image loaded, or add a new image block and add an image
  3. Select the image block and notice the blue border shows around the image
  4. Press the image while the image block is selected and notice the image will be viewed in fullscreen.
  5. Press outside the boundaries of the image in fullscreen mode, or swipe the image down to exit the fullscreen image preview.

*Note you can also test rotating the device while in fullscreen preview, but there is a known issue that will cause the image preview to close on rotate when testing in WordPress Android on a phone running API 28 - See WordPress Android issues 10227 and 1333

Screenshots

Fade in to fullscreen image preview:
fullscreen-preview-v0

Types of changes

New feature - this code updates the image block of the mobile editor so that users can do a fullscreen preview of the image that they are adding.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.

@talldan talldan added [Type] Enhancement A suggestion for improvement. First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository labels Aug 21, 2019
@mtias mtias added the Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) label Aug 21, 2019
Copy link
Contributor

@Tug Tug left a comment

Choose a reason for hiding this comment

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

Great work so far @cameronvoell !
I had a few comments on the code that I think we should address, otherwise this is very good for a v1 imo

@cameronvoell cameronvoell force-pushed the add/mobile-fullscreen-image-preview branch from 64e25d4 to 3e8445a Compare August 26, 2019 20:29
@cameronvoell cameronvoell force-pushed the add/mobile-fullscreen-image-preview branch from 3e8445a to 3b3f645 Compare August 26, 2019 20:32
…removing utils class since it is not being used anywhere else.
…r, perhaps becauser we have no children tap events we need to worry about. Fixed animationOutTiming constant.
@cameronvoell cameronvoell force-pushed the add/mobile-fullscreen-image-preview branch from 1c0cca8 to 9d3bbab Compare August 26, 2019 22:09
etoledom and others added 3 commits August 28, 2019 10:31
* [RNMobile] Fix crash when adding separator

* Build: remove global install of latest npm since we want to use the paired node/npm version (WordPress#17134)

* Build: remove global install of latest npm since we want to use the paired node/npm version
* Also update travis to remove --latest-npm flag

* [RNMobile] Try dark mode (iOS) (WordPress#17067)

* Adding dark mode component implemented on list and list block

* Adding DarkMode handling to RichText, ToolBar and SafeArea

* Mobile: Using DarkMode as HOC

* iOS DarkMode: Modified colors on block list and block container

* iOS DarkMode: Improved Header Toolbar colors

* iOS DarkMode: Removing background from buttons

* iOS DarkMode warning and unsupported

* iOS DarkMode: MediaPlaceholder

* iOS DarkMode: BottomSheets

* iOS DarkMode: Inserter

* iOS DarkMode: DefaultBlockAppender

* iOS DarkMode: PostTite

* Update hardcoded colors with variables

* iOS DarkMode: Fix bottom-sheet cell value color

* iOS DarkMode: More - PageBreak - Add Block Here

* iOS DarkMode: Better text color

* iOS Darkmode: Code block

* iOS DarkMode: HTML View

* iOS DarkMode: Improve colors on SafeArea

* Fix toolbar not avoiding keyboard regression

* Fix native unit tests

* Fix gutenberg-mobile unit tests

* Adding RNDarkMode mocks

* RNMobile: Fix crash when viewing HTML on iOS

* [RNMobile] Remove toolbar from html view

* [RNMobile] Fix MaxListenersExceededWarning caused by dark-mode event emitter (WordPress#17186)

* Fix MaxListenersExceededWarning caused by dark-mode event emitter

* Checking for setMaxListeners trying to avoid CI error

* Adding remove listener to DarkMode HOC

* DarkMode: Binding this.onModeChanged to `this`

* DarkMode: Adding conditional needed to pass UI Tests on CI

* Fix focus title on new posts regression (WordPress#17180)

* BottomSheet: Setting DashIcon color directly when theme is default (light) (WordPress#17193)
* First working version of the MediaText component for native mobile

* Fix adding a block to an innerblock list

* Disable mediaText on production

* MediaText native: improve editor visuals

* Move BlockToolbar from BlockList to Layout

* Remove BlockEditorProvider from BlockList and add native version of EditorProvider to Editor. Plus support InsertionPoint and BlockListAppender

* Update BlockMover for native to hide if locked or if it's the only block

* Make the vertical align button work, add more styling options for toolbar buttons

* Make sure registerCoreBlocks does not break in production

* Copy docblock comment from the web version for registerCoreBlocks

* Fix focusing on the media placeholder

* Only support adding image for now

* Update usage of MediaPlaceholder in MediaContainer

* Enable autoScroll for just the out most block list

* Fix JS Unit tests

* Roll back to IconButton refactor and fix tests

* Fix BlockVerticalAlignmentToolbar buttons style on mobile

* Fix thing for web and ensure ariaPressed is always passed down

* Use AriaPressed directly to style SVG on mobile

* Update snapshots
@cameronvoell
Copy link
Member Author

Closed in favor of native solution. More info: wordpress-mobile/gutenberg-mobile#1286 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.