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

Stories: Upgrade Kanvas version #18967

Merged
merged 9 commits into from
Jul 8, 2022
Merged

Stories: Upgrade Kanvas version #18967

merged 9 commits into from
Jul 8, 2022

Conversation

twstokes
Copy link
Contributor

@twstokes twstokes commented Jun 30, 2022

Description

This PR upgrades Kanvas to version 1.4.3 which fixes a couple issues:

Testing

App doesn't crash with many images

  1. Using WordPress / Jetpack iOS, tap the FAB button to create a new Story post
  2. Add 15+ images
  3. Tap the arrow at the top right to start the publishing process
  4. Expect no crash
  5. Enter a title and tap "Publish Now"
  6. Expect that publishing succeeds and the post can be loaded in the app and on the web (note: the amount of time before the post is published will depend on how many media items were chosen, their size, and the device's network connection speed)

Text layers aren't lost if publishing is cancelled

  1. Using WordPress / Jetpack iOS, tap the FAB button to create a new Story post
  2. Take a picture with the camera or add images from the media library
  3. Add text to the current item shown (tap the Aa button at the bottom left)
  4. Tap the arrow at the top right to start the publishing process
  5. Cancel publishing by swiping down on the sheet
  6. Expect that the text added remains on the view

Known issue

No output is shown after cancelling the publishing process and trying to add a photo from the camera.

  1. Add media
  2. Tap the arrow at the top right to start the publishing process
  3. Cancel publishing by swiping down on the sheet
  4. Tap the "+" at the bottom right to capture media from the camera
  5. Observe a gray screen and no camera output

A Github issue will be added if it doesn't already exist.

Regression Notes

  1. Potential unintended areas of impact

    • Publishing Stories
    • Adding media to a story
    • Adding text layers to media
  2. What I did to test those areas of impact (or what existing automated tests I relied on)

    • Manual tests
  3. What automated tests I added (or what prevented me from doing so)

    • None

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding unit tests for my changes.
  • 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.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Jun 30, 2022

You can test the changes in Jetpack from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr18967-4243877 on your iPhone

If you need access to App Center, please ask a maintainer to add you.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Jun 30, 2022

You can test the changes in WordPress from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr18967-4243877 on your iPhone

If you need access to App Center, please ask a maintainer to add you.

@twstokes twstokes added this to the 20.2 ❄️ milestone Jun 30, 2022
@twstokes twstokes marked this pull request as ready for review June 30, 2022 16:40
@@ -9,7 +9,7 @@ struct SiteIconPickerView: View {
var onDismiss: (() -> Void)? = nil

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just FYI - the Site Icon Picker is not currently in production, so these low-risk changes won't affect anything.

@twstokes
Copy link
Contributor Author

twstokes commented Jun 30, 2022

👋 @SiobhyB! I'm awaiting reviews on the Kanvas side, but I thought I'd add you as a reviewer in case you wanted to look at things ahead of time. Thanks!

@twstokes twstokes modified the milestones: 20.2 ❄️, 20.3 Jul 5, 2022
Copy link
Contributor

@SiobhyB SiobhyB left a comment

Choose a reason for hiding this comment

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

Thank you for this @twstokes 🙇‍♀️

I went through the testing steps you outlined, the story block test cases, and tried to break things through random steps. All looks good to me, with the original bugs squashed!

I know you're still waiting on an approval for tumblr/kanvas-ios#140, but wanted to share my testing so far and clear the way for you for when that PR gets approved. If changes are needed, feel welcome to re-request a review :D

Podfile Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

WPiOS Crashed When Adding Media to a Story Post
3 participants