News:

Support the VirtueMart project and become a member

Main Menu

MultiVariant Bug in Mobile Devices.

Started by sandstorm, November 16, 2016, 23:58:41 PM

Previous topic - Next topic

sandstorm

MultiVariant Bug in Mobile Devices.

Initially I thought this was a problem with my template or theme, but after extensive testing in different sites, different version & different server set up, I can reproduce the bug in virtuemart with protostar with other plugins, themes, cache, etc switched off.
Also when testing this in a resized browser of mobile device simulator, there is no bug & it all works OK.
Only see bug in actual mobile devices at small screen size that this problem persists

PROBLEM
When viewing a Multi Variant product on a mobile device (Mainly iOS, its seems 50/50 in android devices), a user selects the 1st variant, but can then not select a 2nd or 3rd as there appears to be nothing populated to select.
This means that the box which read "Add a variant" cannot change to "Add to Cart"

This is the same for multi variant products with 1, 2 or 3 variants - I haven't tried more

Developers I have worked with in testing this have suggested that it appear that a JS file is not loaded properly on 1st load in the iOS device as If we refresh the problematic page Once or Twice, then select the Multi Variants it all works OK.


How to reproduce
In an iOS device (or some android devices) Navigate to one of the links below - If 1st works try others
Select multi variants - Sometimes it works, but most times not

https://www.staging2.lowenergysupermarket.com/lighting/type-of-light/led-light-bulbs/382/gu10-4x1w-short-led-light-bulb-spotlight-les
https://www.staging2.lowenergysupermarket.com/lighting/type-of-light/led-light-bulbs/116/led-candle-light-bulb-les
https://www.staging2.lowenergysupermarket.com/lighting/type-of-light/led-light-bulbs/819/7w-led-360-bulb-non-dimmable-3-les
https://www.staging2.lowenergysupermarket.com/lighting/lighting-controls/occupancy-sensors/253/wall-mounted-pir-occupancy-sensor-les
https://www.staging2.lowenergysupermarket.com/water-heating/water-services/tap-aerators/71/tap-aerator-les


Thing we checked tested already
;

  • Waited for page to load fully
    cleared site,server,browser cache
    Tested on 3.0.16 & 3.0.18 - We have an older version on 3.0.9, which it works OK
    Tested PHP7 or PHP5.6
    Tested on Multiple iOS Phone sized devices & Samsung Galaxy


Screenshots

https://www.dropbox.com/s/ju5b7ryh2lasp7u/problem.jpg?dl=0
https://www.dropbox.com/s/s9q5p0pkizzr0c1/after.jpg?dl=0
J3.6.4 / PHP7.0.12
VM3.0.16

Studio 42

Hi,

I have do a test becasue i had already similar problem.
This is the javascript log in a desktop browser, when you have the bug, each time you click on a option.
QuoteUncaught TypeError: ((n.event.special[g.origType] || (intermediate value)).handle || g.handler).apply is not a function(...)
Uncaught TypeError: ((n.event.special[g.origType] || (intermediate value)).handle || g.handler).apply is not a function(...)
Uncaught TypeError: ((n.event.special[g.origType] || (intermediate value)).handle || g.handler).apply is not a function(...)

Tested with google chrome, last release

When i check the problem. The options are attached as an object to the select list change action, but one function is missing called at this line :
Virtuemart.cvFind = function(event) {
On checking the code, i mean that the "container" is not found in some case.
What i don't understand, is
Quotewhile(!container.hasClass('product-field-display') && runs<=maxruns){
      container = container.parent();
      runs++;
   }
you have jquery function to do that. jquery closest see https://api.jquery.com/closest/
so in your case it's certainly
container  = jQuery(this).closest('.product-field-display').parent();

It's same for this code :
QuotejQuery(container).find('.cvselection:checked').each(function() {
      selection[selection.length] = jQuery(this).val();
      found = true;
   });
perhaps not exactly the right code, but similar is right
Quote
var tmpSelect = jQuery(container).find('.cvselection:checked');
if(tmpSelect.length) selection[selection.length] = jQuery(container).find('.cvselection:checked').val();
else selection[selection.length] = jQuery(container).find('.cvselection').eq(0).val();

For the other part of the script, i had change more too

But at the end, what is needed ? the seleted link or not ?
Because this get reloaded  each time and the other value are never compared, i think this can certainly work better with  5 or 6 lines of codes only.




Milbo

Quote from: Studio 42 on November 17, 2016, 10:54:45 AM
:
Virtuemart.cvFind = function(event) {
On checking the code, i mean that the "container" is not found in some case.
What i don't understand, is
Quotewhile(!container.hasClass('product-field-display') && runs<=maxruns){
      container = container.parent();
      runs++;
   }
you have jquery function to do that. jquery closest see https://api.jquery.com/closest/

yeh but it works only when the DOM IS the class, not if it HAS the class. sandstorm did you override the customfield.php in sublayouts? There are some changes which refer to mobile devices. Actually I think it is already added vm3.0.18. You may also load the vm3.0.18.3 and test the "legacy layout" option
Should I fix your bug, please support the VirtueMart project and become a member
______________________________________
Extensions approved by the core team: http://extensions.virtuemart.net/

Studio 42

Max, the script inject unwanted codes in the select list.
Try many time, some time you can see the result on refresh and change an option.
Function is replaced by your "variants" object. I think that this depend if the loading is to slow and dom and JS is not loaded in the right order or something so.
It's why it's hard to have the bug on a desktop computer and you have it more time on a mobile.
the js error report
Uncaught TypeError: ((n.event.special[g.origType] || (intermediate value)).handle || g.handler).apply is not a function(...)
is because this is not anymore a dom element but considared as a javascript object, after this multi variant stop to work.

A really simple solution(but i know you hate this) is to change the script and add the URL in the Joomla select.genericlist options using data-url attribute.
If you do so, you can completly remove the multi variants script, on getting the URL from the selected option and doing a standard ajax request.

A sample script is for eg. (incomplet of course)
$('body').on('change','.multivariant', function(){
var url = $(this).data('url');
// ajax call url

});


This need no extra values in javascript and you can remove half of the current code for multi variants in front.

Milbo

This does not work Patrick, because the you need the combination of dropdowns. the strength of the MV is when you have more than one dropdown, that is the reason this does not workThe genchild work that way. They store the url directly in the dropdown
Should I fix your bug, please support the VirtueMart project and become a member
______________________________________
Extensions approved by the core team: http://extensions.virtuemart.net/

Milbo

Are you sure you have the version with


vmJsApi::addJScript('cvfind');


and

vmJsApi::addJScript('cvselvars'.$hash,$j,true,false,false,$hash);

sublayouts/customfield around line 234 ???
Should I fix your bug, please support the VirtueMart project and become a member
______________________________________
Extensions approved by the core team: http://extensions.virtuemart.net/

Studio 42

I have checked my theory, and adding data-url atribute in the option is working and need only this little script in all for product details and category or modules.
if('productdetails' != vRequest::getCmd('view') or !VmConfig::get ('jdynupdate', TRUE)){
$jsRedirect = "
Virtuemart.isUpdatingContent = true;
window.top.location.href = url";
} else {
$jsRedirect = "
url = url.replace(/amp;/g, '');
Virtuemart.setBrowserNewState(url);
Virtuemart.updateContent(url);";
}
$j = "
jQuery('.cvselection').on('change', function(e){
var url =jQuery(this).find(':selected').data('url');
if(url!= '#'){".$jsRedirect."}
});
";
// $hash = md5(implode('',$tags));
$hash='';
vmJsApi::addJScript('cvselvars'.$hash,$j,false);


Not this is only needed 1 time for all multivariantes in same page.


This is the select list needed for eg.
<select id="customProductData_302_300cvard0" name="field[302][300][customfield_value]" class="vm-chzn-select cvselection no-vm-bind chzn-done" data-dynamic-update="0" style="min-width: 70px; display: none;">
<option value="" data-url="#">-- Select --</option>
<option value="46.0000" data-url="/virtuemart-fr/index.php/en/product-variants/multi-variant-2-htm" selected="selected">46.0</option>
<option value="51.0000" data-url="/virtuemart-fr/index.php/en/product-variants/multi-variant-6-htm">51.0</option>
<option value="56.0000" data-url="/virtuemart-fr/index.php/en/product-variants/multi-variant-11-htm">56.0</option>
<option value="61.0000" data-url="/virtuemart-fr/index.php/en/product-variants/multi-variant-15-htm">61.0</option>
</select>


This not change the option value itself,  so it's full compatible for the cart.

This not need the cvfind script anymore.

Studio 42


Milbo

Again Patrick, when you have only one option, use generic child variants.

The multivariant CANNOT have the url in the dropdown, because the combination of dropdownS! determine the product. If there is a timing problem, we have to understand how we can avoid it. Not by just removing the function
Should I fix your bug, please support the VirtueMart project and become a member
______________________________________
Extensions approved by the core team: http://extensions.virtuemart.net/

Studio 42

See the cart screenshot, it work. It's  a real example with my changes, not a test.

I only added the little script and added data-url attributes in generic.list options.
This mean that the form values are untouched. and this get send to the cart as before. So it's full backward compatible, and if a template has an overide, it work too but use the old cvfind script.

Milbo

Still you do not understand, generic child:

Parent has one Option
- child a
- child b
- child c

Multivariant can have up to 5 options, lets show 2 options

Parent with option 1 and 2

1-1 => child a
1-2 => child b
2-1 => child c
2-2 => child d

The system is written so, that when we remove automatic reloading using a dropdown, that you could set any dropdown and fire the logic (search a child with the desired parameters). You are right, we directly reload, when use one ONE dropdown, so we could rewrite it so, that any dropdowns contains the links, which fit for the other dropdowns. But this works only correct, when we allow to change only one parameter at once.

Yeah and it not so easy to write it.

But actually, that is not the topic. The topic is that it works on Desktop but has sometimes problems on mobile devices. The team worked hard before vm3.0.18 on this problem. We called it the "safari" problem, because we got it only with safari based browsers. It turned out that the main reason was using jQuery within an environment already set to jQuery. The other problem was, that the vmJsApi::addJScript('cvselvars'.$hash,$j,true,false,false,$hash); used before asynchron loading, which worked well on the Desktop, but not on safaria browsers and also created problems on mobiles, when the connection was not fast enough.

Should I fix your bug, please support the VirtueMart project and become a member
______________________________________
Extensions approved by the core team: http://extensions.virtuemart.net/

Studio 42

I completly understand the multivariant, the only difference, is that multivariante need the values.
BUt as i worte before, the value are same as before, only data-url atribute is added in the option.If you don't know what data-XX is see : https://api.jquery.com/data/ or https://developer.mozilla.org/fr/docs/Web/Guide/HTML/Using_data_attributes

Milbo

No, the multivariant does not take the values from there. I explained it exactly above. The multivariant checks for the combination and searches the right URL from an array.

$jsArray[] = '["'.$url.'","'.implode('","',$variants).'"]';

I told you, we can change it your way, but only because we fire the refresh of the page automatically, when we change one dropdown. But the system is written so, that you could change more than one dropdown at once and fire the new variant.

So when we have more than one dropdown, the urls must be the right ones, for the next dropdowns. I cant see that you take care of this in your code. So all what you did is to create a kind of generic child variant out of it. Then you can directly use the js of the generic childs. It works this way, besides it does not use data-url. I have not a problem to change it to data-url, but this needs a correct patch

Just try your solution with two dropdowns. You may understand then.
Should I fix your bug, please support the VirtueMart project and become a member
______________________________________
Extensions approved by the core team: http://extensions.virtuemart.net/

Milbo

Your method would can only work in certain circumstances.

The MV allows also that the first dropdown determines the options of the second.
Should I fix your bug, please support the VirtueMart project and become a member
______________________________________
Extensions approved by the core team: http://extensions.virtuemart.net/

Studio 42

Quote from: Milbo on November 21, 2016, 19:58:15 PM
No, the multivariant does not take the values from there. I explained it exactly above. The multivariant checks for the combination and searches the right URL from an array.

$jsArray[] = '["'.$url.'","'.implode('","',$variants).'"]';

I told you, we can change it your way, but only because we fire the refresh of the page automatically, when we change one dropdown. But the system is written so, that you could change more than one dropdown at once and fire the new variant.

So when we have more than one dropdown, the urls must be the right ones, for the next dropdowns. I cant see that you take care of this in your code. So all what you did is to create a kind of generic child variant out of it. Then you can directly use the js of the generic childs. It works this way, besides it does not use data-url. I have not a problem to change it to data-url, but this needs a correct patch

Just try your solution with two dropdowns. You may understand then.
I have do the test with the vm samples using 2 dropdown. And it work.
When you refresh the page(using ajax or not) the values are not send in the link, only the new URL.
After loading all old values are rmoved and new are recalculated. SO the only need of the values are in the cart display.

I don't removed the code to set the options, so it wrok exactly as before, but the URLs are integrate to the options, so this don't need any array of values and attaching the on change to the select list for all the products work, because the datas (url here) are get directly from selected option.

If you check right, the current way the select list is set in php, it depend the product id and not the values itself. and after the preselected values are set depending the product id.