Variation fields can permanently disappear during ajax replacement

I also had a similar issue with Commerce Quick Purchase module. If only an attribute on the Add to cart is ajax refreshed then autocomplete field of the module is also somehow changed and don't work anymore. For now, as a quick and dirty solution I've made this:

http://cgit.drupalcode.org/commerce_quick_purchase/tree/src/QuickPurchas...

Not sure what the actual cause is but as it noticed already the DC attempts to render an empty field on ajax refresh. Which field? That is the question! :)

it's corrects the issue, when the first variation doesn't have the field filled

Fix PHP typo

Thanks for the patch, it indeed fixes the vanishing fields issue. However, the duplicating divs still persist. That probably needs to be fixed wherever the variation fields are actually rendered?
Also, I'm not sure if we need to implement out own isEmpty-check, when there is helper functions like Element::isEmpty, though that one only works in special cases. Not sure if there is a better one we could use.

Alright, I've looked at this a little more closely. The patch was causing PHP notices because in replaceRenderedFields it checks $content, but the variable is called $rendered_field here. Furthermore, this whole second part isn't needed (which is why the patch worked despite the mistake), because all content in replaceRenderedFields has already run though renderField, where the fix was added initially.
So I removed the offending portion and reworded some of the documentation.

I also looked into the wrapper div issue, and it seems we can't really do much about that in contrib. The offending code lies in core/misc/ajax.js where it reads:

var $newContentWrapped = $('<div></div>').html(response.data);
var $newContent = $newContentWrapped.contents();

if ($newContent.length !== 1 || $newContent.get(0).nodeType !== 1) {
  $newContent = $newContentWrapped;
}

Basically, it's checking whether the inserted content is only a single html element, else it creates a plain wrapper div. However, the output of the field template (at least when using the classy theme) contains whitespace before the actual element (and potentially twig debug comments if enabled), which means $newContent contains a whitespace text node before the actual element. So it just wraps it in a div.

Thanks for the patch, this help to have something which work, even if i think we still have work to make a commit.

I make some improvements, but i still have some issues and see other cases this couldn't be enough.

1/ We still have the <div> nested issue
2/ I add a test in isEmpty() in case field has a #markup but no item. I think we should think about other cases like templates (no markup, no items), which also should display something even if the template return nothing.
3/ I add a hide class. I ok the markup is empty, but in case some css apply to this class, we should not display this (i guess)

#9.1 there is an active core-issue for the nesting issue #736066: ajax.js insert command sometimes wraps content in a div, potentially producing invalid HTML and other bugs

Some tests are missing, we should be sure #prefix and #suffix exists.

Marked #3006862: List price doesn't display after switching to a variation without a list price as a duplicate.

#13 working for me

Will this patch work with Commerce 8.x-2.10 ?

@zincdesign yes

That's great thanks. #13 Working for me.

The div nesting issue was fixed by Drupal 8.6.0, which we require.

Looking into this issue I found problems with performance and handling fields that are hidden on the entity display, fixed in #3010225: Optimize ProductVariationFieldRenderer performance. This patch now needs a reroll.

Implementation notes:
- We should be able to use Element::children() to check if the render array is empty.
- Setting #theme/#type to container should be an easy way to ensure a div.

Updated patch.

1) Still missing the main test coverage (ensuring that switching the variation renders the field). No good place to add it to right now, it seems.

2) There's some code duplication between renderFields() and renderField()

3) An empty div has a height, so it changes the way the page looks. We could explode either using a span, or adding an additional class that can be targetted with CSS.

Adding tests.

I think this redirect test tends to fail randomly.

This should be the final patch.

Renamed and cleaned up the functional test, since it's now doing more than testing the view mode.
Eliminated the code duplication in ProductVariationFieldRenderer.
Decided to keep the container as-is (which means keeping its height) for now. It does prevent a sliding effect when a new value appears. People can target the div with :empty in CSS.

Thanks, everyone!