WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#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
Milestone: 4.3.1 Priority: normal
Severity: normal Version: 4.3
Component: Customize Keywords: has-patch commit fixed-major
Focuses: javascript Cc:

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 5 years ago.
33428.2.diff (614 bytes) - added by westonruter 5 years ago.
Ensure that any completeCallback gets called
33428.3.diff (1.2 KB) - added by westonruter 5 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.


5 years ago

#2 @netweb
5 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
5 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
5 years ago

#4 @westonruter
5 years ago

#33518 was marked as a duplicate.

#5 @westonruter
5 years ago

#33434 was marked as a duplicate.

#6 @westonruter
5 years ago

#33420 was marked as a duplicate.

#7 @westonruter
5 years ago

  • Milestone changed from Awaiting Review to 4.3.1
  • Owner set to westonruter
  • Status changed from new to accepted

#8 @westonruter
5 years ago

  • Focuses javascript added
  • Keywords has-patch added

#9 @westonruter
5 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 b  
    26032624                                _( constructs ).each( function ( activeConstructs, type ) {
    26042625                                        api[ type ].each( function ( construct, id ) {
    26052626                                                var active = !! ( activeConstructs && activeConstructs[ id ] );
    2606                                                 construct.active( active );
     2627                                                if ( active ) {
     2628                                                        construct.activate();
     2629                                                } else {
     2630                                                        construct.deactivate();
     2631                                                }
    26072632                                        } );
    26082633                                } );
    26092634                        } );
Version 0, edited 5 years ago by westonruter (next)

@westonruter
5 years ago

Ensure that any completeCallback gets called

#10 @nikeo
5 years ago

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

#11 @westonruter
5 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.


5 years ago

#13 @westonruter
5 years ago

#33538 was marked as a duplicate.

#14 @westonruter
5 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
5 years ago

  • Resolution set to fixed
  • Status changed from accepted to 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
5 years ago

  • Keywords commit fixed-major added
  • Resolution fixed deleted
  • Status changed from closed to 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
5 years ago

#33494 was marked as a duplicate.

#18 @westonruter
5 years ago

  • Resolution set to fixed
  • Status changed from reopened to 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.