Fix depends field not working for radio elements #11539

jahvi · 2017-10-18T13:44:00Z

Fixes issue with configuration fields not toggling when <depends> is used on radio elements.

Description

Currently the <depends> field only works when used on elements of select type, this PR aims to replicate the same functionality for fields with radios type.

Fixed Issues (if relevant)

<depends> field doesn't work in system.xml for "radios" fields #9360: field doesn't work in system.xml for "radios" fields

Manual testing scenarios

Create field with type radios and corresponding field that dependant on one of its values as mentioned it <depends> field doesn't work in system.xml for "radios" fields #9360
Navigate to backend and find configuration page with created field.
Toggle radio options on and off, dependant field should hide/show as expected.

Contribution checklist

Pull request has a meaningful description of its purpose
All commits are accompanied by meaningful commit messages
All new or changed code is covered with unit/integration tests (if applicable)
All automated tests passed successfully (all builds on Travis CI are green)

magento-cicd2 · 2017-10-18T13:44:02Z

All committers have signed the CLA.

jahvi · 2017-10-18T13:44:10Z

This is just an initial implementation of the fix the promote discussion, although it works I'm open to suggestions on best practices.

osrecio

Extract line this.trackChange(null, idTo, elementsMap[idTo]); out of if/else and check test with codeStyle: Travis(289515174)

osrecio · 2017-10-20T07:32:36Z

lib/web/mage/adminhtml/form.js

@@ -402,6 +402,17 @@ define([
                        );
                        this.trackChange(null, idTo, elementsMap[idTo]);


Extract this line out of if/else

osrecio · 2017-10-20T07:32:46Z

lib/web/mage/adminhtml/form.js

+                                'change',
+                                this.trackChange.bindAsEventListener(this, idTo, elementsMap[idTo])
+                            );
+                        }
                        this.trackChange(null, idTo, elementsMap[idTo]);


Extract this line out of if/else

jahvi · 2017-10-20T14:31:02Z

@osrecio Thanks for the comments, I went ahead and made the recommended changes.

osrecio · 2017-10-20T18:15:40Z

Looks ok for me

omiroshnichenko · 2017-11-17T06:18:04Z

lib/web/mage/adminhtml/form.js

+                    values = valuesFrom[idFrom].values;
+                    fromId = $(idFrom + values[0]);
+                    radioFrom = $$('[name="' + fromId.name + '"]:checked');
+                    isInArray = values.indexOf(radioFrom[0].value) != -1; //eslint-disable-line


Add check on radioFrom.length > 0(in case when none is selected), make strict equality and remove eslint-disable-line

jahvi · 2017-11-17T09:49:54Z

@omiroshnichenko Thanks for your input I just made the proposed changes.

omiroshnichenko · 2017-11-21T13:43:34Z

lib/web/mage/adminhtml/form.js

+                        // Check if radio button
+                        values = elementsMap[idTo][idFrom].values;
+                        fromId = $(idFrom + values[0]);
+                        radioFrom = $$('[name="' + fromId.name + '"]');


After testing was detected that additional check must be added

radioFrom = fromId ? $$('[name="' + fromId.name + '"]') : false;

omiroshnichenko · 2017-11-21T13:44:12Z

lib/web/mage/adminhtml/form.js

+                } else {
+                    values = valuesFrom[idFrom].values;
+                    fromId = $(idFrom + values[0]);
+                    radioFrom = $$('[name="' + fromId.name + '"]:checked');


Same check here

radioFrom = fromId ? $$('[name="' + fromId.name + '"]:checked') : [];

Fixes magento#9360

jahvi · 2017-11-21T14:10:08Z

@omiroshnichenko Thanks just updated the PR with the changes

WaPoNe · 2018-02-12T18:46:01Z

Great work @jahvi

@okorshenko this fix should be used also in 2.3-develop branch. Why don't I see it?

okorshenko · 2018-02-14T20:53:13Z

It is on the way to 2.3-develop branch. Testing now

…pends field is not exist

jahvi mentioned this pull request Oct 18, 2017

<depends> field doesn't work in system.xml for "radios" fields #9360

Closed

osrecio reviewed Oct 20, 2017

View reviewed changes

jahvi force-pushed the fix/depends-not-working-radio-inputs branch from 1e7d9d4 to 5ac8f9f Compare October 20, 2017 14:30

magento-engcom-team added 2.2.x Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release labels Nov 7, 2017

okorshenko added this to the November 2017 milestone Nov 7, 2017

okorshenko added the Release Line: 2.2 label Nov 7, 2017

okorshenko assigned magento-engcom-team Nov 7, 2017

omiroshnichenko self-requested a review November 17, 2017 06:15

omiroshnichenko reviewed Nov 17, 2017

View reviewed changes

magento-team added the Progress: needs update label Nov 17, 2017

jahvi force-pushed the fix/depends-not-working-radio-inputs branch 2 times, most recently from 707a952 to 427bdca Compare November 17, 2017 09:48

omiroshnichenko self-assigned this Nov 17, 2017

magento-team removed the Progress: needs update label Nov 17, 2017

omiroshnichenko reviewed Nov 21, 2017

View reviewed changes

magento-team added the Progress: needs update label Nov 21, 2017

Fix depends field not working for radio elements

0031a1c

Fixes magento#9360

jahvi force-pushed the fix/depends-not-working-radio-inputs branch from 427bdca to 0031a1c Compare November 21, 2017 14:09

magento-team added Progress: accept and removed Progress: needs update labels Nov 21, 2017

okorshenko merged commit 0031a1c into magento:2.2-develop Nov 22, 2017

okorshenko pushed a commit that referenced this pull request Nov 22, 2017

MAGETWO-83401: Fix depends field not working for radio elements #11539

235737f

jahvi deleted the fix/depends-not-working-radio-inputs branch November 22, 2017 19:38

magento-engcom-team added the Fixed in 2.2.x The issue has been fixed in 2.2 release line label Nov 22, 2017

okorshenko unassigned magento-engcom-team Nov 22, 2017

magento-engcom-team pushed a commit that referenced this pull request Apr 28, 2018

Fix depends field not working for radio elements #11539: case when de…

da5692c

…pends field is not exist

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix depends field not working for radio elements #11539

Fix depends field not working for radio elements #11539

jahvi commented Oct 18, 2017 •

edited by AlexWorking

magento-cicd2 commented Oct 18, 2017 •

edited

jahvi commented Oct 18, 2017

osrecio left a comment •

edited

osrecio Oct 20, 2017

osrecio Oct 20, 2017

jahvi commented Oct 20, 2017

osrecio commented Oct 20, 2017

omiroshnichenko Nov 17, 2017 •

edited

jahvi commented Nov 17, 2017

omiroshnichenko Nov 21, 2017

omiroshnichenko Nov 21, 2017

jahvi commented Nov 21, 2017 •

edited

WaPoNe commented Feb 12, 2018

okorshenko commented Feb 14, 2018

		@@ -402,6 +402,17 @@ define([
		);
		this.trackChange(null, idTo, elementsMap[idTo]);

Fix depends field not working for radio elements #11539

Fix depends field not working for radio elements #11539

Conversation

jahvi commented Oct 18, 2017 • edited by AlexWorking

Description

Fixed Issues (if relevant)

Manual testing scenarios

Contribution checklist

magento-cicd2 commented Oct 18, 2017 • edited

jahvi commented Oct 18, 2017

osrecio left a comment • edited

Choose a reason for hiding this comment

osrecio Oct 20, 2017

Choose a reason for hiding this comment

osrecio Oct 20, 2017

Choose a reason for hiding this comment

jahvi commented Oct 20, 2017

osrecio commented Oct 20, 2017

omiroshnichenko Nov 17, 2017 • edited

Choose a reason for hiding this comment

jahvi commented Nov 17, 2017

omiroshnichenko Nov 21, 2017

Choose a reason for hiding this comment

omiroshnichenko Nov 21, 2017

Choose a reason for hiding this comment

jahvi commented Nov 21, 2017 • edited

WaPoNe commented Feb 12, 2018

okorshenko commented Feb 14, 2018

jahvi commented Oct 18, 2017 •

edited by AlexWorking

magento-cicd2 commented Oct 18, 2017 •

edited

osrecio left a comment •

edited

omiroshnichenko Nov 17, 2017 •

edited

jahvi commented Nov 21, 2017 •

edited