Make WordPress Core

Opened 5 years ago

Closed 3 years ago

#48896 closed defect (bug) (fixed)

When enqueued wp-color-picker, toggle response of each widget panel is unresponsive on customizer.

Reported by: inc2734's profile inc2734 Owned by: audrasjb's profile audrasjb
Milestone: 6.0 Priority: normal
Severity: normal Version:
Component: Customize Keywords: has-patch has-testing-info commit assigned-for-commit
Focuses: Cc:

Description

Confirmed to occur on Android Chrome.
It was not reproduced on iOS Safari, Mac Chrome / Safari.
Perhaps opening and closing will ignite at the same time.

Reproduction code

/themes/widget-test
├ style.css
├ functions.php
└ index.php

style.css

/**
 * Theme name: widget test
 */

functions.php

<?php
add_action(
	'widgets_init',
	function() {
		register_sidebar(
			[
				'name'          => __( 'sidebar', 'widget-test' ),
				'id'            => 'widget-test-sidebar',
				'before_widget' => '<div id="%1$s" class="c-widget %2$s">',
				'after_widget'  => '</div>',
				'before_title'  => '<h2 class="c-widget__title">',
				'after_title'   => '</h2>',
			]
		);
	}
);

add_action(
	'admin_enqueue_scripts',
	function() {
		wp_enqueue_script( 'wp-color-picker' );
	}
);

index.php

<head>
	<?php wp_head(); ?>
</head>
<body>
	<?php dynamic_sidebar( 'widget-test-sidebar' ); ?>
	<?php wp_footer(); ?>
</body>

Attachments (3)

48896.mov (331.2 KB) - added by dlh 5 years ago.
A widget acting though it was clicked twice.
android-chrome.mp4 (343.8 KB) - added by inc2734 5 years ago.
I tapped at regular intervals. At that time, DOM is shining.
48896.diff (518 bytes) - added by costdev 3 years ago.

Download all attachments as: .zip

Change History (21)

#1 @kmix39
5 years ago

Reproduced :
-Android Firefox

Did not reproduce :
-iOS Firefox

#2 @dlh
5 years ago

  • Keywords reporter-feedback added
  • Version 5.3 deleted

Hi @inc2734, and thanks for this report and the detailed replication steps.

Are you able to provide a screen capture of this behavior?

Also, can you confirm whether the same bug occurs with widgets on the standalone /wp-admin/widgets.php page?

I don't have an Android device to test with, but I do see some strange behavior when using the replication code in the Firefox device emulator. A "touch" action is treated like a double-click: An open widget collapses then expands again, or vice versa.

@dlh
5 years ago

A widget acting though it was clicked twice.

#3 @inc2734
5 years ago

Are you able to provide a screen capture of this behavior?

I will prepare it later.

Also, can you confirm whether the same bug occurs with widgets on the standalone /wp-admin/widgets.php page?

/wp-admin/widgets.php works correctly.

@inc2734
5 years ago

I tapped at regular intervals. At that time, DOM is shining.

#4 @dlh
5 years ago

  • Keywords needs-patch added; reporter-feedback removed
  • Milestone changed from Awaiting Review to Future Release

Thanks for the screen capture, @inc2734!

From what I can see in the video, the widget form does expand or collapse some of the time, but not every time it was tapped. Is that correct?

I've been unable to replicate the issue so far in the browser emulator, and, as I said, I don't have an Android device available. So, it seems as though this issue needs a patch, but it would be helpful for whoever picks up this ticket to try to describe the cause in a device-independent way, if that's even possible.

#5 @celloexpressions
4 years ago

  • Keywords needs-testing added
  • Milestone changed from Future Release to 5.8

This is likely fixed/invalidated by the block based widget editor in 5.8. Milestoning for testing/confirmation.

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


4 years ago

#7 @desrosj
4 years ago

  • Milestone changed from 5.8 to 5.9

While the traditional widget screen is being replaced with a block based widget editor, the Classic Widgets plugin will be maintained for a bit of time in the same way the Classic Editor was (expect an announcement to set expectations soon).

With that in mind, enhancement and feature requests will most likely no longer be considered. But bug tickets are still welcome, so long as they don't require too many resources distracting away from block based editing.

Since the 5.8 deadline is today and this ticket still requires testing to confirm it is reliably reproducible before patching, I'm going to punt this one. I'll move to 5.9 with the hopes we can resolve this sooner rather than later so that Classic Widget users can see this fixed.

#8 @hellofromTonya
3 years ago

  • Milestone changed from 5.9 to 6.0

With no activity and 5.9 Beta 1 happening in ~ 18 hours, moving this to 6.0 for further investigation.

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


3 years ago

#10 @costdev
3 years ago

Testing on Firefox (Windows) DevTools (Galaxy S20 Linux), I was able to produce incorrect multi-click behaviour.

  1. Navigate to Appearance > Customize > Widgets.
  2. Double-click on the widget header.

Expected behaviour was either:

  1. Open the widget and ignore the second click.
  2. Open the widget and close it again.

Actual behaviour: The widget opened, closed and opened in quick succession.

This appears to be due to the expanded class not being applied until after the animation is complete.

First click:

  • Toggle button: "My aria-expanded isn't true, open the widget."
  • Widget: "I don't have the expanded class yet, open me."

Second click while still animating:

  • Toggle button: "My aria-expanded is true, close the widget."
  • Widget: "I don't have the expanded class yet, open me."

In this situation, the toggle button is the source of truth.

Changing if ( expanded ) to if ( $toggleBtn.attr( 'aria-expanded' ) === 'false' ) is what fixed it for me. I'll attach a patch in just a moment.

@costdev
3 years ago

#11 @costdev
3 years ago

  • Keywords has-patch added; needs-patch removed

To test the patch:

  1. Run these commands:
    grunt patch:48896
    npm run build:dev
    
  1. Navigate to Appearance > Customize > Widgets.
  2. Double-click/Double-tap the widget title. It should just open instead of rubber-banding.

#12 @costdev
3 years ago

  • Keywords has-testing-info added

This ticket was mentioned in Slack in #core-test by boniu91. View the logs.


3 years ago

#14 @Boniu91
3 years ago

Working as expected.

Test Report

Env

  • WordPress 6.0-alpha-52448-src
  • Firefox 93 + DevTools (Galaxy S20 Linux)
  • Windows 10
  • Theme: Astra
  • Plugin: Classic Widgets

Steps to test

  1. Go to Customize - Widgets
  2. Select sidebar with at least one widget
  3. Double click/tap on the widget header
  4. Widget is opened, there's no doubled action, etc.

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


3 years ago

#16 @peterwilsoncc
3 years ago

  • Keywords commit added

#17 @audrasjb
3 years ago

  • Keywords assigned-for-commit added; needs-testing removed
  • Owner set to audrasjb
  • Status changed from new to assigned

Thanks for the patch and tests 👍

#18 @audrasjb
3 years ago

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

In 53267:

Widgets: Fix Classic Widgets screen toggle response on small screens.

This changeset updates the toogle logic to fix Widget's responsive behavior on Classic Widgets screen.

Props inc2734, kmix39, dlh, desrosj, costdev, Boniu91.
Fixes #48896.

Note: See TracTickets for help on using tickets.