Opened 10 years ago
Closed 2 years ago
#31334 closed enhancement (wontfix)
Customizer JS API should handle container removal from document and provide destroy callback
Reported by: | Aniruddh | Owned by: | westonruter |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | 4.1 |
Component: | Customize | Keywords: | has-patch needs-refresh needs-unit-tests |
Focuses: | javascript, administration | Cc: |
Description
When we do
wp.customize.section.remove('my_section')
or
wp.customize.control.remove('my_control')
The API should handle removing their container from document, too. The current process requires you to execute this before removing a section (or control):
wp.customize.section('my_section').container.remove()
Attachments (4)
Change History (24)
#2
@
10 years ago
- Keywords has-patch needs-refresh added
- Milestone changed from Awaiting Review to Future Release
@Aniruddh: can you update your patch to:
- Use tabs instead of spaces for indentation
- Add brackets around the if statement, per the coding standards (even though it's one-line)
- Please patch relative to the root of WordPress, ie the patch should be something like
src/wp-admin/js/customize-base.css
@westonruter: this looks good to me and makes sense for the API to handle, do you see any issues?
@
10 years ago
Sorry, I have less knowledge about how this whole thing works hence the previous dirty patch. I have updated the patch like you said and attached it here.
This ticket was mentioned in Slack in #core-customize by aniruddh. View the logs.
10 years ago
#4
@
10 years ago
- Keywords needs-refresh removed
- Milestone changed from Future Release to 4.3
- Owner set to westonruter
- Status changed from new to assigned
Patch looks good, thanks. This'll go nicely with #30741.
#5
@
10 years ago
Yes, this makes sense. However, I think the logic shouldn't be added to the base class's method wp.customize.Values.prototype.remove()
since there's nothing about container
mentioned at all in customize-base.js
.. Instead, this should be added in the subclasses that actually have this property. And then customize-widgets.js
can also take advantage of this to remove its own control.container.remove()
. See 31334.2.diff for what I have in mind.
Needs QUnit testing.
This ticket was mentioned in Slack in #core-customize by celloexpressions. View the logs.
9 years ago
#8
@
9 years ago
- Milestone changed from 4.3 to Future Release
I'm not going to have time, so yeah, punt.
This ticket was mentioned in Slack in #core-customize by celloexpressions. View the logs.
8 years ago
#11
@
8 years ago
As discussed in Slack, we may need to use jQuery#detatch
instead of jQuery#remove
, since remove
will destroy added event listeners, causing the control to be inert if it is attempted to be re-added after it is removed.
This ticket was mentioned in Slack in #core-customize by westonruter. View the logs.
8 years ago
#13
@
8 years ago
For an example situation where this would be useful, see https://github.com/xwp/wp-customize-featured-content-demo/blob/8bf4d65/js/featured-items-panel.js#L514-L522
#14
@
8 years ago
- Keywords needs-unit-tests added; has-unit-tests removed
This needs to be revisited. If someone wants to refresh the patch and look further into any complications.
One thing to note: if a control gets removed from wp.customize.control
then it should probably be forbidden from being re-added, the deferreds will have already resolved and any attached event listeners would have been destroyed. That is, unless we use jQuery#detatch
instead of jQuery#remove
.
#15
@
7 years ago
- Summary changed from Customizer JS API should handle container removal from document to Customizer JS API should handle container removal from document and provide destroy callback
Just as there is a ready
method that controls can define to add custom logic to set up a control, so too there needs to be a destroy
method that is called when the control is removed from wp.customize.control
. This is where calls like ReactDOM.unmountComponentAtNode()
would be made.
#18
@
7 years ago
See an example implementation here: https://github.com/xwp/wp-customize-react-street-address-control/commit/9495e8a9774a9a0891fe65512bcd52a3bc7065c2
Line 334-335