Skip to content

Conversation

@rachelmcr
Copy link
Contributor

@rachelmcr rachelmcr commented Nov 8, 2022

Closes: #8046

Description

This PR enables product preview for all stores (instead of only WordPress.com-hosted stores) by using a frame-nonce parameter in the preview URL. The same preview approach is used for all stores, to reduce complexity.

It also updates the Preview button so it's always visible on products that can be previewed (new and draft products), and just disabled when the product is a new blank product. This is better for accessibility, so the button doesn't appear/disappear from the navigation bar.

Caveat: I can't come up with a good way to distinguish template products from new blank products, so the preview button is only enabled once you make changes to the template product details. (This is the same behavior as before.)

Changes

  • Removes the authentication step from displayProductPreview() and just adds the required query parameters (preview and frame-nonce) to the preview URL.
  • Removes the check for new blank products when action buttons are added to the navigation bar, and moves that to a separate check shouldEnablePreviewButton() to determine if the Preview button should be enabled.

Testing

  1. Build and run the app in debug or alpha mode.
  2. Select a store not hosted on WordPress.com.
  3. Go to the Products tab and add a new product. Confirm the Preview button is visible but disabled, and it is enabled once you start making changes.
  4. Tap the Preview button. Confirm the new product is saved and you can see a preview on your store.

Screenshots

Simulator.Screen.Recording.-.iPhone.14.Pro.-.2022-11-08.at.17.51.19.mp4

Submitter Checklist

Update release notes:

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

@rachelmcr rachelmcr added status: feature-flagged Behind a feature flag. Milestone is not strongly held. feature: products onboarding Related to onboarding new users to manage products labels Nov 8, 2022
@rachelmcr rachelmcr added this to the 11.2 milestone Nov 8, 2022
@peril-woocommerce
Copy link

Warnings
⚠️ This PR is assigned to a milestone which is closing in less than 2 days Please, make sure to get it merged by then or assign it to a later expiring milestone

Generated by 🚫 dangerJS

@rachelmcr rachelmcr requested a review from Ecarrion November 8, 2022 18:34
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Nov 8, 2022

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

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

@ealeksandrov ealeksandrov self-assigned this Nov 9, 2022
Copy link
Contributor

@ealeksandrov ealeksandrov left a comment

Choose a reason for hiding this comment

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

All test cases work well and look good for me 🚀
✅ self-hosted site
✅ wp.com site
✅ wp.com site with mapped domain

///
func shouldEnablePreviewButton() -> Bool {
!(formType == .add && !hasUnsavedChanges())
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Good idea about toggling the button state instead of hiding it!

Copy link
Contributor

Choose a reason for hiding this comment

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

We may want to change this condition due to the template experiment, as a new template product would be considered as a new product without any changes.

I think we need to add a check for the "blank" state.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we need to add a check for the "blank" state.

I agree, but I haven't come up with a good solution for it yet. This PR doesn't change whether you can preview new template products (it just makes the preview button visible but disabled, instead of not appearing at all), so I'm going to go ahead and merge it. I've opened a new issue #8071 so we can follow up with a fix for new template products.

@ealeksandrov ealeksandrov removed their assignment Nov 9, 2022
@rachelmcr rachelmcr merged commit ea5de4a into trunk Nov 9, 2022
@rachelmcr rachelmcr deleted the issue/8046-preview-with-frame-nonce branch November 9, 2022 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature: products onboarding Related to onboarding new users to manage products status: feature-flagged Behind a feature flag. Milestone is not strongly held.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Product Preview: WebView Authentication (self-hosted sites)

5 participants