- Issue created by @pfrenssen
- 🇺🇦Ukraine khiminrm
Maybe it could work if it was fixed in Drupal Core https://www.drupal.org/project/drupal/issues/3100473 →
- Merge request !14Separate the add to cart confirmation message from other messages. → (Open) created by khiminrm
- 🇺🇦Ukraine khiminrm
Created MR with solution to separate the add to cart confirmation message from other messages before render.
- Status changed to Needs review
3 months ago 3:43pm 3 April 2024 - 🇺🇦Ukraine khiminrm
Maybe another approach we could use - save message in private storage and render it in custom block?
- 🇮🇱Israel jsacksick
Private storage is a good idea, but I wouldn't save the whole HTML there, I'd simply save the data that I need to render the message, so just the order item ID?
Then whenever the private storage is accessed, you can just load the order item, and render the theme function?
But yeah, using the messenger is kind of hackish (that is how it was done in D7 as well as far as I remember). - First commit to issue fork.
- 🇺🇦Ukraine khiminrm
I've created another on MR with the solution to use the private store https://git.drupalcode.org/project/commerce_add_to_cart_confirmation/-/m...
- 🇮🇱Israel jsacksick
hm... Helper is a weird name, maybe too generic... And why would you need to have a method to delete the order item ID?
Also, surprised it isn't the case already, but we night need to store the quantity that was added as well in case somebody decides to not use Views for the cart confirmation.
Also the MR has code commented out.
We can maybe keep the code simple, just store directly into the private temp store...Once you introduce an interface then you have to maintain it and you can no longer break BC so you have to think carefully :p. - 🇺🇦Ukraine khiminrm
@jsacksick, I used delete method to delete stored ID and do not show the same message multiple times.
Could you, please, explain this:
We can maybe keep the code simple, just store directly into the private temp store
We shouldn't use additional service and just store value in the event subscriber, right?
- 🇮🇱Israel jsacksick
Introducing a new service is not bad idea in general, the problem is that with the current methods defined, there's no room for additional arguments in the future.
Let's say you'd like to store the quantity that was added, or even the order ID, then your methods are suddenly obsolete.
Maybe you can go with:CartConfirmationManager as the service name?
Then define the following methods:- recordAddToCart(), from here you can accept an array (quantity, order_item_id)...
- getCartItemInfo()
- clear()
- 🇺🇦Ukraine khiminrm
@jsacksick thanks for your help! I've improved code. Please, review.
- 🇮🇱Israel jsacksick
Reviewed, what was the problem with page attachments again?
- 🇺🇦Ukraine khiminrm
@jsacksick regarding page attachments alter. When I tested there were caching problem. So there were no dialog until clearing a site cache. I can re-test
- 🇺🇦Ukraine khiminrm
re-tested. when using code inside commerce_add_to_cart_confirmation_page_attachments() it doesn't work - need to clear cache to see the dialog.