Opened 17 months ago
Closed 2 months ago
#37269 closed enhancement (fixed)
Introduce removed event for wp.customize.Values collection
| Reported by: |
|
Owned by: |
|
|---|---|---|---|
| Milestone: | 4.9 | Priority: | normal |
| Severity: | normal | Version: | 3.4 |
| Component: | Customize | Keywords: | has-patch has-unit-tests commit |
| Focuses: | javascript | Cc: |
Description (last modified by westonruter)
Notifications were added to Customizer controls in #34893. Notifications are re-rendered when the control.notifications collection (a wp.customize.Values) has a wp.customize.Notification added or removed with the code as follows (via wp.customize.Control#initialize):
// After the control is embedded on the page, invoke the "ready" method. control.deferred.embedded.done( function () { /* * Note that this debounced/deferred rendering is needed for two reasons: * 1) The 'remove' event is triggered just _before_ the notification is actually removed. * 2) Improve performance when adding/removing multiple notifications at a time. */ var debouncedRenderNotifications = _.debounce( function renderNotifications() { control.renderNotifications(); } ); control.notifications.bind( 'add', function( notification ) { wp.a11y.speak( notification.message, 'assertive' ); debouncedRenderNotifications(); } ); control.notifications.bind( 'remove', debouncedRenderNotifications ); control.renderNotifications(); control.ready(); });
Notice the multi-line comment. We can't re-render the notifications at a remove event because this event gets triggered just before it is removed, somewhat unexpectedly. We should therefore introduce a removed event that gets triggered after the removal so that workarounds using debounce or defer aren't required.
Submitted patch should include unit test.
Change History (13)
#1
@westonruter
17 months ago
- Description modified (diff)
- Keywords needs-unit-tests added
#2
@westonruter
15 months ago
- Milestone changed from Future Release to 4.7
- Version changed from 4.6 to 3.4
This ticket was mentioned in Slack in #core by jorbin. View the logs.
14 months ago
This ticket was mentioned in Slack in #core-customize by celloexpressions. View the logs.
14 months ago
#5
@celloexpressions
14 months ago
- Milestone changed from 4.7 to Future Release
#8
@westonruter
13 months ago
- Keywords good-first-bug removed
- Owner set to delawski
- Status changed from new to assigned
@delawski nice. I added a code review comment.
#9
@delawski
13 months ago
@westonruter Thank you for the review. I've added the condition as you suggested and also wrote additional unit test to cover case you've mentioned in the comment.
#10
@westonruter
13 months ago
- Keywords has-unit-tests commit added; needs-unit-tests removed
- Owner changed from delawski to westonruter
- Status changed from assigned to reviewing
Excellent. This can go into 4.8 as soon as trunk it open for enhancements.
#11
@westonruter
2 months ago
- Milestone changed from Future Release to 4.9
- Status changed from reviewing to accepted
I'm going to add this as part of #35210.
#12
@westonruter
2 months ago
#13
@westonruter
2 months ago
- Resolution set to fixed
- Status changed from accepted to closed
In 41374:
Can still make 4.7 if there's a patch, but we need a patch for now.