Skip to content

[Gutenberg] Implements a call from gutenberg editor to native image fullscreen preview#12979

Merged
cameronvoell merged 10 commits intodevelopfrom
gutenberg/image-fullscreen-preview-native
Nov 21, 2019
Merged

[Gutenberg] Implements a call from gutenberg editor to native image fullscreen preview#12979
cameronvoell merged 10 commits intodevelopfrom
gutenberg/image-fullscreen-preview-native

Conversation

@cameronvoell
Copy link
Copy Markdown
Contributor

@cameronvoell cameronvoell commented Nov 19, 2019 •
edited
Loading

Fixes gb-mobile/1286

gutenberg-mobile PR: wordpress-mobile/gutenberg-mobile#1597
gutenberg PR: WordPress/gutenberg#18618

More Info (Description, Steps)

See gutenberg-mobile PR: wordpress-mobile/gutenberg-mobile#1597

ios-fullscreen-video

Update release notes:

  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@peril-wordpress-mobile
Copy link
Copy Markdown

peril-wordpress-mobile bot commented Nov 19, 2019 •
edited
Loading

You can trigger an installable build for these changes by visiting CircleCI here.

@cameronvoell cameronvoell added the Gutenberg Editing and display of Gutenberg blocks. label Nov 20, 2019
@cameronvoell cameronvoell added this to the 13.8 milestone Nov 20, 2019
This was referenced Nov 20, 2019
Merged
Merged
@cameronvoell cameronvoell mentioned this pull request Nov 20, 2019
Closed
@cameronvoell cameronvoell changed the title Implemented a call from gutenberg editor to native image fullscreen preview [Gutenberg] Implemented a call from gutenberg editor to native image fullscreen preview Nov 20, 2019
@cameronvoell cameronvoell changed the title [Gutenberg] Implemented a call from gutenberg editor to native image fullscreen preview [Gutenberg] Implements a call from gutenberg editor to native image fullscreen preview Nov 20, 2019
@cameronvoell cameronvoell marked this pull request as ready for review November 20, 2019 03:39
cameronvoell
cameronvoell commented Nov 20, 2019
View reviewed changes
WordPress/Classes/ViewRelated/Gutenberg/GutenbergViewController.swift
}
}

func gutenbergDidRequestFullscreenImage(with mediaUrl: URL) {
Copy link
Copy Markdown
Contributor Author

@cameronvoell cameronvoell Nov 20, 2019

Choose a reason for hiding this comment

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

Inspired by

WordPress-iOS/WordPress/Classes/ViewRelated/Media/MediaItemViewController.swift

Lines 189 to 195 in 3231410

private func presentImageViewControllerForMedia() {
let controller = WPImageViewController(media: self.media)
controller.modalTransitionStyle = .crossDissolve
controller.modalPresentationStyle = .fullScreen
self.present(controller, animated: true)
}

SergioEstevao
SergioEstevao approved these changes Nov 20, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@SergioEstevao SergioEstevao 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 @cameronvoell !

It was just missing the support for images coming from private sites.
I took the liberty to add support for those, I hope you don't mind.

etoledom
etoledom reviewed Nov 20, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@etoledom etoledom left a comment

Choose a reason for hiding this comment

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

It works great overall! Nice job @cameronvoell 🎉

I noticed a small glitch, not sure if you or @SergioEstevao can reproduce it:

When the cross-disolve transition starts, the controller that is seen at the background is the PostList instead of the Gutenberg editor.

Same when dismissing the image (is more noticeable when dismissing by a simple tap), what appears on the animation is the Blog List, until the animation finishes and it shows the editor again (assuming that the editor was opened from the Blog List controller).

cross-disolve

I looked a bit on this and I found this solution:

  • Set controller.modalPresentationStyle to overCurrentContext instead of fullScreen
  • Set the top-most navigation controller as the presentation context:
    navigationController?.definesPresentationContext = true

In this way, all the animation happens over the AztecNavigationController (used to present Gutenberg), and it looks good :)

cameronvoell and others added 2 commits November 20, 2019 13:33
@cameronvoell @etoledom
Update RELEASE-NOTES.txt
6716fc8 
Making release notes description of fullscreen preview from image block more clear.

Co-Authored-By: etoledom <etoledom@icloud.com>
@cameronvoell
Adjusted transition to WPImageViewController so we no longer see a fl…
08988b9 
…ash of post list.
@cameronvoell
Copy link
Copy Markdown
Contributor Author

cameronvoell commented Nov 20, 2019

In this way, all the animation happens over the AztecNavigationController (used to present Gutenberg), and it looks good :)

Nice catch, and thanks for explaining the fix. Implemented here: 08988b9

See video of the better transition now:
video-better-transition

etoledom
etoledom approved these changes Nov 21, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@etoledom etoledom left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thank you for the update @cameronvoell
Is working perfectly now. Great job 🎉

And thank you @SergioEstevao for helping with private sites :)

@cameronvoell cameronvoell merged commit 5d88bd8 into develop Nov 21, 2019
@cameronvoell cameronvoell mentioned this pull request Nov 21, 2019
Closed
1 task
@cameronvoell cameronvoell deleted the gutenberg/image-fullscreen-preview-native branch February 14, 2020 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SergioEstevao SergioEstevao SergioEstevao approved these changes

+2 more reviewers

@houndci-bot houndci-bot houndci-bot left review comments

@etoledom etoledom etoledom approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Gutenberg Editing and display of Gutenberg blocks.

Projects

None yet

Milestone

13.8 ❄️

Development

Successfully merging this pull request may close these issues.

Add Fullscreen Preview to Image Options

4 participants

@cameronvoell @SergioEstevao @houndci-bot @etoledom