-
Notifications
You must be signed in to change notification settings - Fork 31
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
Help Finding Product Type Selector #430
base: trunk
Are you sure you want to change the base?
Conversation
/home/runner/work/woocommerce-accommodation-bookings/woocommerce-accommodation-bookings/assets/js/utils.js 12:22 error Replace `⏎↹↹.closest('.product.product-type-accommodation-booking')` with `.closest('.product.product-type-accommodation-booking')⏎↹↹` prettier/prettier ✖ 1 problem (1 error, 0 warnings) 1 error and 0 warnings potentially fixable with the `--fix` option.
/home/runner/work/woocommerce-accommodation-bookings/woocommerce-accommodation-bookings/assets/js/utils.js 12:9 error Insert `(⏎↹↹` prettier/prettier 13:3 error Replace `.length·>·0` with `↹.length·>·0⏎↹)` prettier/prettier ✖ 2 problems (2 errors, 0 warnings) 2 errors and 0 warnings potentially fixable with the `--fix` option.
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.
Code looks good to me but wondering if there's an E2E test we want to add here? If this is truly an issue due to a WordPress or Woo update that changed the markup, I would have expected a failing E2E test to catch this, though that doesn't seem to be the case.
@dkotter I checked and found the following e2e test code that covers checking the existence of the unavailable start day. The E2E test is expected to fail but it requires an investigation. The original issue is that we do not see the unavailable day, rather we see a green/available day; the following test code checks for it. woocommerce-accommodation-bookings/tests/e2e/specs/product.spec.js Lines 194 to 198 in ad98ad6
|
@faisal-alvi I am checking this issue with the trunk branch and it works as expected with trunk. Can you check once if you still able to reproduce with trunk? Recording.675.mp4 |
I tested again in
Please make sure you re-build the project when you move to the trunk branch from this PR branch. If it still does not work, let's plan a 1:1 connection. |
@faisal-alvi I checked again with the older Woo version 8.8.3 as mentioned above and also tried without using fixes of this PR#3755 as having time zone-related fixes but I am still not able to reproduce it. Let's have a 1:1 when you have time. I will try on your setup. |
Issue background:This issue was reported approximately 3 months ago, and recent testing showed it was not reproducible. we tried on various environments, but it was hard to reproduce. After in-depth debugging, it was found that this issue is only reproducible with the TT4 theme and works fine with the Storefront theme, although at the time of reporting, the issue was also happening with the Storefront theme. Please have a look at the screencast below to know more (with trunk/stable branch): Recording.709.mp4The below QA report shows that the current PR fixes the issue for TT4 as well. |
QA/Test Report- ✅Testing Environment -
Test Results - Issue has been fixed and working fine with Storefront as well as Twenty Twenty Four theme. Functional Demo / Screencast - After Fix: Recording.710.mp4Before Fix: Recording.709.mp4Next Step- Ready for UAT. |
Regression+Smoke Test Report- ✅Testing Environment -
Tested with Archive File created via
Please note that this plugin has been tested with the build created by the specified versions ☝🏼 of Composer, Node, and NPM.Status- Working as expected. Ready to merge 🚀 cc: @vikrampm1 |
All Submissions:
Changes proposed in this Pull Request:
After a thorough debugging and investigation, it is found that the search for the selector for the product type was incorrectly coded (or maybe the HTML changed in recent WP/Woo releases). This PR tweaks the code so that the selector is found and the proper class is applied to the dates, making them display as fully booked when required, ultimately fixing the reported issue (#417).
Note: in the reported issue it is mentioned that it occurs only when the "Display visitor's local time" setting is enabled but it was occurring regardless of this setting.
Closes #417.
Steps to test the changes in this Pull Request:
Changelog entry