-
-
Notifications
You must be signed in to change notification settings - Fork 87
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
Refactor offcanvas cart #329
Conversation
Variable product disabled button still shows loader on click
Update Refactor offcanvas branch
Hide View Cart
Update 5.2.1
Upstream main
Upstream main
Upstream main
Upstream main
Upstream main
Upstream main
Planning to take a good look at this this weekend. Did you also check how well this works when Ajax is disabled? At work we use the bootScore woocommerce resources, and sometimes we need to disable ajax to make more advanced and custom systems. |
Bug found: when going back a page, the cart isn't loaded with content. |
Also, I see we're still using the default woocommerce alerts. Any chance we could kinda revamp that? The icon is set with the font version of Font Awesome, although I haven't used those for almost years now 😂 I've completely switched to the JS version, and needed to fall back on the woocommerce icons even though they don't look as good. |
Urgh, yes. Bug appears in actual ajax-cart https://bootscore.me/shop/ as well. But it seems that this is a Woo mini-cart widget issue. Added a second cart widget in sidebar here https://dev.bootscore.me/shop/. Always when you hit the go back button, cart is empty. You must add something new or reload the page. Bug appears even if ajax-cart is completely disabled. Think this has nothing to do with this PR.
FA icons to alerts are added here https://github.com/bootscore/bootscore/blob/main/scss/bootscore/_alerts.scss. I'm open to remove them in a second PR.
Yes, disabling ajax-cart works pretty well. |
We can use a script to force reload when back button is clicked. This one seems to work https://dev.bootscore.me/shop/: // Refresh page on browser back button
// https://stackoverflow.com/questions/43043113/how-to-force-reloading-a-page-when-using-browser-back-button
window.addEventListener("pageshow", function (event) {
var historyTraversal = event.persisted ||
(typeof window.performance != "undefined" &&
window.performance.navigation.type === 2);
if (historyTraversal) {
// Handle page restore.
window.location.reload();
}
}); |
Isn't there a way to just refresh the cart, like a js event? I don't think reloading the page is a good idea. And for the fontawesome part, I know they're being set there, but that only works when you load the font. But the JS version replaces the i tag with an svg so those fonts aren't being loaded. Maybe we could load the svg directly there like bootstrap does. |
Figured out that this happens only on Chrome and Opera. Safari, Edge and Firefox works fine (on my machine). But this is already a known issue woocommerce/woocommerce#32454. This workaround works fine woocommerce/woocommerce#32454 (comment): var isChromium = window.chrome;
if (isChromium) {
$(window).on('pageshow', function (e) {
if (e.originalEvent.persisted) {
setTimeout(function () {
$(document.body).trigger('wc_fragment_refresh');
}, 100);
}
});
} Font Awesome: We had this already here #46 (comment), but nothing happen? |
That solution looks good! Feel free to implement it and I'll take a full look this weekend. About FA, let's have a quick chat about it on TG, a bit easier 😉 |
Fixed in e8bea45 |
Did some more testing, and all looks good! Really impressed by it 😄 I'll be doing some quick formatting tests and then it's ready to be merged. |
Perfect ;-) But we should wait to merge until next week Wednesday, because on Monday a new Woo release with template changes (again) is scheduled. So, we can update templates on Tuesday and do a minor release 5.2.3.1 then. The cart we should ship with Bootstrap 5.3 if stable or beta, agree? Mabye you can have a quick look to #332 and #344, which can be merged now. This are just minor improvements. |
While checking out the code, I noticed the js of the mini cart is getting quite long. Maybe it's a good idea to put it in a file instead of having it inline? And I'll check out the others one first then, as most changes are in the js 👍 |
Yes, it's long. But because JS and PHP is in one file it's very simple to deactivate with one hit. Translation strings will not work that way too if we put JS in an own file. And of course, ajax is direct on the server, no more file request. |
What do you think, should we ship the cart with the next Woo release or wait? |
In that case, would it maybe be cleaner to have the js in a separate file but echo the content directly with a file_get_contents()? That way the code looks a bit cleaner and you still have the direct js in the footer. |
Just did some small js changes, could you quickly check them? Don't have a Woocommerce test environment 😜 Also, you want to wait till bootstrap 5.3 is beta or stable? Knowing their trackrecord that can take quite a while. |
https://dev.bootscore.me/shop/ seems good 👍 |
If you agree, we merge now and release it next week. |
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.
All seems to be working fine! 👍
Want to press the merge button? |
This PR delivers a completely refactored ajax cart and has no breaking changes since 5.2.0.0.
Demo: https://dev.bootscore.me/shop/
Before & After clip: https://www.youtube.com/watch?v=IBsuoKNDrXk
Ajax cart is now compatible with all product types:
Add to cart buttons & Offcanvas cart
Offcanvas loader has been removed and buttons have now a loading-spinner instead. Offcanvas opens when product is added to cart.
Alerts
Offcanvas shows now a success/danger alert when product is added/failed to the cart and removes them when offcanvas closes.
Product page
Alerts are moved out from the top of the page into offcanvas cart. These are the default WooCommerce alerts.
Loop
These alerts are not the WooCommerce alerts, because it seems that WC does not provide alerts in loop. Text can be translated like any other text in the theme.
There is no redirect to product page anymore if adding failed.
Testing
Loop: https://dev.bootscore.me/shop/
Simple product
Variable product
Only 1 in stock / sold individually
Grouped products
Affiliate products
Add to cart buttons in other page content
Click the “Album“ button below the
ul
https://dev.bootscore.me/2012/01/07/template-sticky/.Disable ajax cart
Because all scripts (except the mini cart in navbar) are now located in
ajax-add-to-cart.php
, it's very simply to disable ajax cart function. Disable "Enable AJAX add to cart buttons on archives" in backend WooCommerce > Settings > Products and add the empty function to your child'sfunctions.php
:Known issues
Alert does not work in related products if product page is out of stock https://dev.bootscore.me/product/t-shirt/. This is done by this script which excludes the ajax cart if product is out of stock.
Without these selectors, the “Read more“ loop button will not work.
If a product failed adding in the loop and then went directly to the cart/checkout page, the alert appears again at the top of the page. No solution for that yet.
Closes #215
Closes #319
Closes #277
Fixes #200 (comment)
Fixes #205
Fixes #209
Fixes #270
Fixes #154
Let me know what you think!