Context Navigation

← Previous Ticket
Next Ticket →

#33428 closed defect (bug) (fixed)

Toggling customizer controls based on another control does not work anymore in 4.3

Reported by:	maimairel	Owned by:	westonruter
Priority:	normal	Milestone:	4.3.1
Component:	Customize	Version:	4.3
Severity:	normal	Keywords:	has-patch commit fixed-major
Cc:		Focuses:	javascript

Description

Hi,

Since update v4.3, the toggling of customizer controls based on another customizer control does not work anymore. It works, but gets hidden immediately after the customizer finished loading.

I checked the core code, and nothing has changed so it must be something else. It's this code that I'm talking about, in wp-admin/js/customize-controls.js

// Control visibility for default controls
$.each({
	'background_image': {
		controls: [ 'background_repeat', 'background_position_x', 'background_attachment' ],
		callback: function( to ) { return !! to; }
	},
	'show_on_front': {
		controls: [ 'page_on_front', 'page_for_posts' ],
		callback: function( to ) { return 'page' === to; }
	},
	'header_textcolor': {
		controls: [ 'header_textcolor' ],
		callback: function( to ) { return 'blank' !== to; }
	}
}, function( settingId, o ) {
	api( settingId, function( setting ) {
		$.each( o.controls, function( i, controlId ) {
			api.control( controlId, function( control ) {
				var visibility = function( to ) {
					control.container.toggle( o.callback( to ) );
				};

				visibility( setting.get() );
				setting.bind( visibility );
			});
		});
	});
});

Thank you :)

Attachments (3)

33428.diff (559 bytes ) - added by nikeo 11 years ago.
33428.2.diff (614 bytes ) - added by westonruter 11 years ago.: Ensure that any completeCallback gets called
33428.3.diff (1.2 KB ) - added by westonruter 11 years ago.: Additional change: https://github.com/xwp/wordpress-develop/commit/6f68997198b08fd44bd0b2481e08c8216914ff13

Download all attachments as: .zip

Change History (21)

This ticket was mentioned in Slack in #core by celloexpressions. View the logs.

11 years ago

#2 @netweb
11 years ago

Adding the chat note by @ celloexpressions in Slack https://wordpress.slack.com/archives/core/p1440004003001718

"#33420, #33428 and #33434 are all duplicates. Testing, looks like the code that toggles those is conflicting with their active states. Best solution is probably to do #29948, but we likely need to figure something else out for 4.3.1. Looks like changing static front page also re-calculates the top position of the section (incorrectly),”

Related: #33420 and #33434

Possible Duplicate: #29948

#3 @nikeo
11 years ago

This bug can be reproduced in TwentyFifteen when accessing the Static Front Page customizer section.
=> The visibility of the controls page_for_posts and page_on_front depends upon the show_on_front setting value.
Since 4.3, the 2 dependant controls are always visible in the two following cases (as far as I understand) :

first customizer load
when the preview is refreshed (because the construct.activate() #2631 method is fired when api.previewer.preview is 'ready'

The bug can be fixed in those two cases by adding a check on the args param of the control.onChangeActive(active, args) method to see if the control is unchanged or not.

patch coming

@nikeo
11 years ago

Attachment 33428.diff added

#4 @westonruter
11 years ago

#33518 was marked as a duplicate.

#5 @westonruter
11 years ago

#33434 was marked as a duplicate.

#6 @westonruter
11 years ago

#33420 was marked as a duplicate.

#7 @westonruter
11 years ago

Milestone Awaiting Review → 4.3.1
Owner set to westonruter
Status new → accepted

#8 @westonruter
11 years ago

Focuses javascript added
Keywords has-patch added

#9 @westonruter
11 years ago

I've identified that the regression was introduced in [33610] to fix #33220.

Specifically, the regression was caused by this change:

src/wp-admin/js/customize-controls.js

-              a
                                 _( constructs ).each( function ( activeConstructs, type ) {
                                         api[ type ].each( function ( construct, id ) {
                                                 var active = !! ( activeConstructs && activeConstructs[ id ] );
+                                                construct.active( active );
+                                                if ( active ) {
+                                                        construct.activate();
+                                                } else {
+                                                        construct.deactivate();
+                                                }
                                         } );
                                 } );
                         } );

Version 0, edited 11 years ago by westonruter (next)

@westonruter
11 years ago

Attachment 33428.2.diff added

Ensure that any completeCallback gets called

#10 @nikeo
11 years ago

@westonruter ah yes the completeCallback() must always be fired thanks

#11 @westonruter
11 years ago

Keywords needs-testing added

@maimairel Can you confirm the latest patch fixes your issue?

This ticket was mentioned in Slack in #core-customize by westonruter. View the logs.

11 years ago

#13 @westonruter
11 years ago

#33538 was marked as a duplicate.

@westonruter
11 years ago

Attachment 33428.3.diff added

Additional change: https://github.com/xwp/wordpress-develop/commit/6f68997198b08fd44bd0b2481e08c8216914ff13

#14 @westonruter
11 years ago

Keywords needs-testing removed

I realized the unchanged check also needs to be included in Container.onChangeActive to do the same thing for sections and panels. See 33428.3.diff .

Incidentally, this also resolves #33494.

#15 @westonruter
11 years ago

Resolution → fixed
Status accepted → closed

In 33754:

Customizer: Ensure persistence of unchanged active state for controls, sections, and panels.

Props nikeo, westonruter.
Fixes #33428 for trunk.
See also #33494.

#16 @westonruter
11 years ago

Keywords commit fixed-major added
Resolution fixed
Status closed → reopened

Re-opening for 4.3.1: [33754] needs to be cherry-picked into the 4.3 branch when it is open for business.

#17 @westonruter
11 years ago

#33494 was marked as a duplicate.

#18 @westonruter
11 years ago

Resolution → fixed
Status reopened → closed

In 33941:

Customizer: Ensure persistence of unchanged active state for controls, sections, and panels.

Cherry-picks [33754] onto 4.3 branch.
Props nikeo, westonruter.
Fixes #33428 for 4.3.
See also #33494.

Note: See TracTickets for help on using tickets.

Download in other formats:

Make WordPress Core

Context Navigation

#33428 closed defect (bug) (fixed)

Toggling customizer controls based on another control does not work anymore in 4.3

Description

Attachments (3)

Change History (21)

This ticket was mentioned in ​Slack in #core by celloexpressions. ​View the logs. 11 years ago

#2 @netweb 11 years ago

#3 @nikeo 11 years ago

@nikeo 11 years ago

#4 @westonruter 11 years ago

#5 @westonruter 11 years ago

#6 @westonruter 11 years ago

#7 @westonruter 11 years ago

#8 @westonruter 11 years ago

#9 @westonruter 11 years ago

src/wp-admin/js/customize-controls.js

@westonruter 11 years ago

#10 @nikeo 11 years ago

#11 @westonruter 11 years ago

This ticket was mentioned in ​Slack in #core-customize by westonruter. ​View the logs. 11 years ago

#13 @westonruter 11 years ago

@westonruter 11 years ago

#14 @westonruter 11 years ago

#15 @westonruter 11 years ago

#16 @westonruter 11 years ago

#17 @westonruter 11 years ago

#18 @westonruter 11 years ago

Download in other formats:

This ticket was mentioned in Slack in #core by celloexpressions. View the logs.

11 years ago

#2 @netweb
11 years ago

#3 @nikeo
11 years ago

@nikeo
11 years ago

#4 @westonruter
11 years ago

#5 @westonruter
11 years ago

#6 @westonruter
11 years ago

#7 @westonruter
11 years ago

#8 @westonruter
11 years ago

#9 @westonruter
11 years ago

@westonruter
11 years ago

#10 @nikeo
11 years ago

#11 @westonruter
11 years ago

This ticket was mentioned in Slack in #core-customize by westonruter. View the logs.

11 years ago

#13 @westonruter
11 years ago

@westonruter
11 years ago

#14 @westonruter
11 years ago

#15 @westonruter
11 years ago

#16 @westonruter
11 years ago

#17 @westonruter
11 years ago

#18 @westonruter
11 years ago