WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 3 years ago

#32882 closed enhancement (fixed)

Customizer search Widgets events

Reported by: afercia Owned by: afercia
Milestone: 5.1 Priority: normal
Severity: normal Version: 4.2
Component: Administration Keywords: has-patch commit
Focuses: javascript Cc:

Description

The events to trigger a search in the available Widgets in api.Widgets.AvailableWidgetsPanelView.events should be paired with what was done for the available Menu Items search, see 32824

See https://core.trac.wordpress.org/ticket/26600#comment:59 for some background.

Attachments (2)

32882.diff (451 bytes) - added by dlh 5 years ago.
32882.2.diff (4.0 KB) - added by afercia 3 years ago.

Download all attachments as: .zip

Change History (14)

#1 @valendesigns
6 years ago

  • Owner set to valendesigns
  • Status changed from new to assigned

I'll pick this one up tomorrow.

#2 @valendesigns
6 years ago

  • Type changed from defect (bug) to enhancement

Actually, I'll work on this early 4.4 as this is technically an enhancement and not a bug from what I can tell.

#3 @afercia
6 years ago

  • Keywords 4.4-early added

#4 @afercia
6 years ago

Related: #32768.

#5 @westonruter
6 years ago

  • Keywords 4.4-early removed

#6 @afercia
6 years ago

There's now a new pattern in core which uses feature detection to determine whether inputs should use the keyup or input event, see [34078] and [36379]. As I see it, this should become standard.

if ( 'oninput' in document.createElement( 'input' ) ) {
	inputEvent = 'input';
} else {
	inputEvent = 'keyup';
}

Any chance to see some progress here? :) cc @ocean90

@dlh
5 years ago

#7 @dlh
5 years ago

  • Keywords has-patch added; needs-patch removed

Since the last comments here, the Customizer has dropped support for IE 8. As I understand https://core.trac.wordpress.org/ticket/26600#comment:59 and other similar discussions (e.g. #38021), IE 8 support was the only reason to continue using keyup, so 32882.diff removes it.

#8 @westonruter
5 years ago

  • Owner changed from valendesigns to afercia
  • Status changed from assigned to reviewing

#9 @afercia
5 years ago

Thanks @dlh ! @westonruter at this point maybe the keyup event can be removed from api.Menus.AvailableMenuItemsPanelView.events too?

@afercia
3 years ago

#10 @afercia
3 years ago

  • Component changed from Customize to Administration
  • Milestone changed from Future Release to 5.1

Right. keyup was there only to support IE 8. 32882.2.diff refreshes the previous patch and also removes the keyup event used in the cases mentioned above:

  • from api.Menus.AvailableMenuItemsPanelView.events
  • the one used in [34078] (together with the feature detection bit)
  • the one used in [36379] (together with the feature detection bit)

#11 @pento
3 years ago

  • Keywords commit added

32882.2.diff is good to commit.

#12 @pento
3 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 44539:

Admin: Don't use the keyup event in addition to the input event.

The keyup event was used to provide support for IE8, where which doesn't support the input event. As we dropped IE8 support some time ago, this was simply adding unnecessary complexity and double-event triggers.

Props dlh, afercia.
Fixes #32882.

Note: See TracTickets for help on using tickets.