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.

Sign up for GitHub

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

Jump to bottom

Stories: iPad Support #16031

Closed
wants to merge 8 commits into from
Closed

Stories: iPad Support #16031

wants to merge 8 commits into from

Conversation

bjtitus
Copy link
Contributor

@bjtitus bjtitus commented Mar 6, 2021 •
edited
Loading

Adds iPad support for Stories.

Related PRs

Kanvas: tumblr/kanvas-ios#112

Changes

  • Kanvas now supports iPad by fixing rotation and layout bugs.
  • Removes to disable Stories for iPad (EditorCreationError and other related code)
  • Changes the discard action sheet to an alert on iPad
  • Uses confirmButton to display the prepublishing sheet as a popover.
  • Changes the modal presentation style to overFullScreen so that it mirrors the iPhone presentation.
  • Fixes a resizing bug in the Prepublishing sheet to allow for proper resizing based on the footer contents.

Testing

  • Verify that the "Story post" option is listed in the bottom sheet when tapping the FAB
  • Create a Story
  • Capture media from your device's camera. Ensure that rotation and capture functionality work as expected.
  • Add media from your device's library. Ensure media is properly displayed in a 9:16 aspect ratio centered in the screen.
  • Ensure that posting the content works as expected.

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.

@peril-wordpress-mobile
Copy link

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

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

@peril-wordpress-mobile
Copy link

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

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

@bjtitus
Reference Kanvas iPad commit
8cdabda
@bjtitus
Autosize Prepublishing Sheet + remove iPad case
f18f2c4 
- autosize the footer view of the prepublishing sheet
- set preferred content size after layout pass
- remove special iPad case so it respects the prepublishing size
@bjtitus bjtitus added this to the 17.3 milestone Apr 21, 2021
@bjtitus
Merge branch 'develop' into stories/ipad
c667335 
# Conflicts:
#	Podfile.lock
@bjtitus bjtitus self-assigned this Apr 30, 2021
@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Apr 30, 2021 •
edited
Loading

Fails
🚫 Podfile: reference to a commit hash
Warnings
⚠️ This PR is assigned to a milestone which is closing in less than 4 days Please, make sure to get it merged by then or assign it to a later expiring milestone

Generated by 🚫 dangerJS

@bjtitus bjtitus marked this pull request as ready for review April 30, 2021 20:47
@jkmassel
Copy link
Contributor

jkmassel commented May 3, 2021

👋 We're freezing 17.3 today, so this PR is being bumped to 17.4

@jkmassel jkmassel modified the milestones: 17.3, 17.4 May 3, 2021
danielebogo
danielebogo approved these changes May 4, 2021
View reviewed changes
Copy link
Contributor

@danielebogo danielebogo left a comment

Choose a reason for hiding this comment

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

Hey @bjtitus code looks good to me.

But I found some problem during the test. Not sure if its related to this PR though so I'm gonna approve it.

  • On iPad/iPhone: If I select more than one image (or shoot more than 1) and I tap the arrow to post, the app crashes. (below the screenshot)
  • On iPhone: if I select an image and then tap the arrow to post it, when I navigate back and I tap the + to add one more image, the camera doesn't work anymore and I can only import images from my camera roll
  • On iPad: same scenario I described above, when I dismiss the popover to post, I can not add a new image because the + button is not enabled.

Screenshot 2021-05-04 at 15 50 24

@jkmassel
Copy link
Contributor

👋 We're freezing 17.4 today, so this PR is being bumped to 17.5

@jkmassel jkmassel modified the milestones: 17.4, 17.5 May 17, 2021
@jkmassel
Copy link
Contributor

We're cutting 17.5 today, so this PR is being bumped to 17.6

@jkmassel jkmassel modified the milestones: 17.5, 17.6 May 31, 2021
@bjtitus bjtitus modified the milestones: 17.6, 17.7 Jun 3, 2021
@mokagio mokagio mentioned this pull request Jun 28, 2021
Closed
4 tasks
@mokagio
Copy link
Contributor

mokagio commented Jun 28, 2021

@bjtitus I'm going to bump this to the next release because we'll be code freezing 17.7 today and this is still a draft.

If this cannot wait two weeks and it's important that this makes it into this release, let me know and we'll organize a new beta once ready. 👍

Also, both this and #16583 have been sitting here for a while. Is there something I can do to help unstuck them? 🤔

@mokagio mokagio modified the milestones: 17.7, 17.8 Jun 28, 2021
@bjtitus bjtitus modified the milestones: 17.8, Someday Jul 9, 2021
@twstokes
Copy link
Contributor

👋 I'm going to close this PR due to lack of activity, but please reopen if this was an error. Thank you.

@twstokes twstokes closed this Aug 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danielebogo danielebogo danielebogo approved these changes

Assignees

@bjtitus bjtitus

Labels
None yet
Projects
None yet
Milestone
Someday
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@bjtitus @jkmassel @mokagio @twstokes @danielebogo