-
Notifications
You must be signed in to change notification settings - Fork 133
[CIAB] Open bookable products in WebView #14765
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
[CIAB] Open bookable products in WebView #14765
Conversation
We removed the duplicated navigation action
mode = ShowProduct(event.productId), | ||
) | ||
) | ||
is OpenProductDetail -> (context.findActivity() as? MainActivity)?.showProductDetail(event.productId) |
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.
One other approach instead of getting the activity would be to use a custom Hilt EntryPoint
and injecting ProductDetailNavigator
here, I don't have a strong opinion about either approach, I just went for the simplest here.
📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.
|
📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.
|
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.
The navigation on the phone works as described, but... it doesn't in the Split Pane layout on the Product List.
Screen_recording_20251016_130629.mp4
I guess, it's something that we want to support as well?
cebd15a
to
5384012
Compare
Thanks @AdamGrzybkowski for the test and for catching the issue. To have better tablet support, I worked on an alternative solution, please check the last commits. The main parts of the new solution are:
Given the navigation is handled now inside The result for tablets now is: Screen_recording_20251017_100101.mp4and for phones: Screen_recording_20251017_101853.mp4Ready for another round. There is an issue about displaying the CIAB nav bar inside the WebView in tablets, I'll start a thread about it and decide if it's something that we want to hide, anyway this is outside of the scope of this PR anyway. |
I forgot to share, there is one additional advantage of this solution, in case the product wasn't cached before, the solution would still work well, we'll show a shimmer loading effect until the product is loaded, then we'll open the WebView: Screen_recording_20251017_102338.mp4This wouldn't have worked with the previous solution. |
e82fb0d
to
ca725ef
Compare
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 like the new approach! The shimmer is a great bonus 👍
One nitpick: The webView has an extra entry animation in the split pane compared to the non-webView fragment Screen_recording_20251017_114633.mp4 |
I took a quick look at this, and couldn't find an easy solution, I tried disabling the animation, but I think it's not better, as this causes the ProdudctDetailFragment content to blink a bit before showing the WebView: Screen_recording_20251017_110702.mp4If you have some ideas we can try please let me know, otherwise I think what we have here is fine, especially given all this sould be a temporary solution until we have a native editor, WDYT? |
ca725ef
to
cbccd3f
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## trunk #14765 +/- ##
============================================
+ Coverage 38.27% 38.29% +0.01%
- Complexity 10010 10017 +7
============================================
Files 2118 2119 +1
Lines 118488 118527 +39
Branches 15824 15826 +2
============================================
+ Hits 45357 45394 +37
- Misses 68917 68919 +2
Partials 4214 4214 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
hmm you're right... The only thing that comes to my mind is to handle switching to WebView within the Not sure if it's worth it. |
Yes, that would be a good alternative, but given how the ProductDetailFragment is already pretty complicated, I think the simplicity and separation of concerns of the current solution is better. |
Issues attributed to commits in this pull requestThis pull request was merged and Sentry observed the following issues:
|
Closes WOOMOB-1320
Description
This PR updates the product navigation to make sure we open bookable products in a WebView.
The URL we use for the products is temporary now.
Note
action_global_productDetailFragment_popUpTo_product_list
, please review commit by commit to better follow the changes.Steps to reproduce
Testing information
The tests that have been performed
The above.
Images/gif
Screen_recording_20251015_222927.mp4
RELEASE-NOTES.txt
if necessary. Use the "[Internal]" label for non-user-facing changes.