Stories: 16.8 Bug Fixes #15996

bjtitus · 2021-03-02T06:06:26Z

Fixes a bug where the Media Picker could be rotated to landscape and mess up the UI. Notice the size of the view controller + tab bar and the "Add" button in the "before" video below:

Rotation.mp4

Fixes a bug when scaling Portrait video to fill the device screen. Note that this does slow down video import a little.

Before	After

Disables Stories on iPad for the 16.8 release.

Testing

Open the Story Editor.
Open the Media Picker using the button in the bottom left.
Rotate the device to landscape and ensure that the status bar remains fixed.
Select a video which was taken in Portrait by your iPhone.
Ensure that the video appears in the correct aspect ratio and is not warped.

iPad

Tap on the FAB from the Blog Details or Post list screens
Ensure that the "Story post" option is not available on iPad
When attempting to edit a story, ensure that a message appears when tapping the edit button.
The Story block should not be available in the block picker when adding a new block.

PR submission checklist:

I have considered adding unit tests where possible.
I have considered adding accessibility improvements for my changes.
I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

This can be done by the Kanvas Metal renderer but we need to work out a few bugs first. For now, we'll rotate these on import.

peril-wordpress-mobile · 2021-03-02T06:06:31Z

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

# Conflicts: # WordPress/Classes/ViewRelated/Gutenberg/GutenbergViewController.swift

peril-wordpress-mobile · 2021-03-02T06:37:03Z

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

aforcier · 2021-03-02T07:07:05Z

Gave this a try, things seem to work as expected 🎉 Found one possible issue.

Confirmed that adding a video filmed in portrait mode used to be warped, and now isn't
Confirmed that nothing breaks when rotating the device (I'm not sure I understood the step - the expected result is that landscape mode is effectively disabled, is that right?)
On an iPad simulator, ensured that "Story post" was not available as an option in the create sheet from either blog details or post list, and confirmed that tapping a story in an existing post shows a message and prevents the story editor from being loaded.

The issue I found is that the story block still shows up in the Gutenberg block picker list on an iPad. We should probably hide it from there as well. However the message that iPad is not supported shows up correctly when attempting to populate the block, so nothing breaks, and we found this to be rarely used on Android - so if it's for some reason difficult/dangerous to implement it's probably fine to leave it in.

aerych

Hola @bjtitus 👋
Looking good! Tested on an iPhone device and iPad simulator. I had some very nitpicky comments code wise but nothing that's a blocker (tho getting rid of the optional would be delightful). from me, and if you decide to make any other changes let me know and I'll quickly re-review. :)

aerych · 2021-03-02T18:21:53Z

WordPress/Classes/ViewRelated/System/WPTabBarController+ShowTab.swift

+                let controller = try StoryEditor.editor(blog: blog, context: ContextManager.shared.mainContext, updated: {_ in }, uploaded: { [weak self] result in
+                    switch result {
+                    case .success:
+                        ()


I know this was an existing expression, but what would you think about changing the () to a break since that's the more expected syntax for a switch/case skip. (Maybe this predates break being added to Swift 🤔)

aerych · 2021-03-02T18:25:22Z

WordPress/Classes/ViewRelated/Gutenberg/GutenbergViewController.swift

+            switch error {
+            case StoryEditor.EditorCreationError.unsupportedDevice:
+                let title = NSLocalizedString("Unsupported Device", comment: "Title for stories unsupported device error.")
+                let message = NSLocalizedString("We are still working on the Stories editor for iPad. In the meantime, please try Stories on your iPhone.", comment: "Message for stories unsupported device error.")


I might be a little overly cautious about the wording here, but I'm wondering if it could run afoul of Apple's "no betas" rule if we get a reviewer that's particularly fussy. Instead of "We are still working on the Stories editor for iPad" would it be safer to say something like "The Stories editor for is not currently available for your iPad"?

aerych · 2021-03-02T18:28:32Z

WordPress/Classes/Services/Stories/WPMediaPicker+MediaPicker.swift

+    /// Applies the `preferredTransform` of the video track.
+    /// - Returns: Returns both an AVMutableComposition containing video + audio and an AVVideoComposition of the rotate video.
+    private func rotate() -> (AVMutableComposition, AVVideoComposition) {
+        let videoTrack = tracks(withMediaType: .video).first!


Maybe this could be tweaked to avoid the implicitly unwrapped optional?

bjtitus added 4 commits March 1, 2021 22:12

Rotate video to preferred orientation

cd72cde

This can be done by the Kanvas Metal renderer but we need to work out a few bugs first. For now, we'll rotate these on import.

Fix landscape rotation bug on iPhone

68caa4f

Disable stories for iPad

cd8d06c

Update Kanvas version for PR

3b1c586

bjtitus added the WP Stories label Mar 2, 2021

bjtitus added this to the 16.8 ❄️ milestone Mar 2, 2021

bjtitus self-assigned this Mar 2, 2021

bjtitus mentioned this pull request Mar 2, 2021

Stories bug fixes for 16.8 release tumblr/kanvas-ios#93

Merged

Merge branch 'release/16.8' into stories/16-8-fixes

1bfe9b2

# Conflicts: # WordPress/Classes/ViewRelated/Gutenberg/GutenbergViewController.swift

Disable stories block on iPad

e7a4f63

bjtitus requested a review from aerych March 2, 2021 16:15

bjtitus marked this pull request as ready for review March 2, 2021 16:15

aerych approved these changes Mar 2, 2021

View reviewed changes

bjtitus added 2 commits March 2, 2021 12:52

Update Kanvas to version 1.2.3

b2cd002

Fixing implicit videoTrack unwrap + PR feedback

266a431

bjtitus merged commit 3f8fece into release/16.8 Mar 2, 2021

bjtitus deleted the stories/16-8-fixes branch March 2, 2021 20:36

fluiddot mentioned this pull request Dec 30, 2022

Disable Story posts when Jetpack features are removed #19823

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Stories: 16.8 Bug Fixes #15996

Stories: 16.8 Bug Fixes #15996

bjtitus commented Mar 2, 2021 •

edited

Loading

peril-wordpress-mobile bot commented Mar 2, 2021 •

edited

Loading

peril-wordpress-mobile bot commented Mar 2, 2021 •

edited

Loading

aforcier commented Mar 2, 2021

aerych left a comment

aerych Mar 2, 2021

aerych Mar 2, 2021

aerych Mar 2, 2021

Stories: 16.8 Bug Fixes #15996

Stories: 16.8 Bug Fixes #15996

Conversation

bjtitus commented Mar 2, 2021 • edited Loading

Testing

iPad

peril-wordpress-mobile bot commented Mar 2, 2021 • edited Loading

peril-wordpress-mobile bot commented Mar 2, 2021 • edited Loading

aforcier commented Mar 2, 2021

aerych left a comment

Choose a reason for hiding this comment

aerych Mar 2, 2021

Choose a reason for hiding this comment

aerych Mar 2, 2021

Choose a reason for hiding this comment

aerych Mar 2, 2021

Choose a reason for hiding this comment

bjtitus commented Mar 2, 2021 •

edited

Loading

peril-wordpress-mobile bot commented Mar 2, 2021 •

edited

Loading

peril-wordpress-mobile bot commented Mar 2, 2021 •

edited

Loading