-
Notifications
You must be signed in to change notification settings - Fork 115
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
Product Form: Support viewing product page for coming soon sites #14931
Product Form: Support viewing product page for coming soon sites #14931
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something strange is happening in my testing store with this PR.
Both with "coming soon" mode enabled or disabled. When opening the web page of every type of product, a Jetpack stats page is loading for me.
While, if I open a published product using trunk
, everything works as expected.
It's like in the version with AuthenticatedWebViewController
, the redirect doesn't work properly.
Hey @pmusolino - can you confirm how you are authenticated in the app? E.g. with WPCom or application password? Can you also try logging out and in again to see if you can reproduce the issue consistently? And does the issue happen to all your test sites? |
I'm authenticated with WPCom
|
Thanks @pmusolino for the review! It looks like the authenticated web view doesn't work if you're authenticated with a WPCom account and open the product page on a self-hosted site. I'll convert this PR back to draft and see how this should be updated. |
Hi @pmusolino - I updated the PR with two changes:
I updated the PR description accordingly - this PR is ready for another look! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested all the cases mentioned in this PR, both with WPCom and Application Password login, and everything works fine now!
Just a nit on release-notes 👍
@@ -7,6 +7,7 @@ | |||
- [*] Receipts: Email receipts can now be sent to customers after failed payments using WooCommerce Stripe 9.1.0+. [https://github.com/woocommerce/woocommerce-ios/pull/14864]. | |||
- [*] Order Creation: orders are always created using the store's currency, not the user's [https://github.com/woocommerce/woocommerce-ios/pull/14907] | |||
- [*] Dashboard: Updated the drag gesture on the Performance chart to show stats instantly. [https://github.com/woocommerce/woocommerce-ios/pull/14906] | |||
- [*] Product Form: Support previewing products on coming-soon stores. [https://github.com/woocommerce/woocommerce-ios/pull/14931] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I wonder if for end users it's more clear "Product Detail" instead of "Product Form".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll rewrite the message on the changelog anyway, I think this note is mostly for us developers. Since changing this will trigger CI again, I'll keep it as-is and merge the PR for the code freeze. I hope that's fine.
Closes: #14924
Description
In an upcoming change, WooCommerce plugin will set all new sites to coming soon mode by default.
This PR updates the web view used in the product form for viewing product page to support viewing products in non-published sites.
Also: updated the background color of the product status badge in the product form.
Steps to reproduce
Testing information
Tested on simulator iPhone 16 Pro iOS 18.2 and confirmed that I can see the product page for all products in both public and coming-soon stores.
Screenshots
RELEASE-NOTES.txt
if necessary.Reviewer (or Author, in the case of optional code reviews):
Please make sure these conditions are met before approving the PR, or request changes if the PR needs improvement: