Customizer: misaligned Widgets reorder button

[afercia]

The Widgets reorder button is a bit too big and a bit misaligned. As far as I see, in 4.2 looks OK so this is probably a small visual regression.

[TomHarrigan]

Fix padding on reorder toggle

[TomHarrigan]

Fix padding on reorder toggle - take 2

[TomHarrigan]

Patch 33260.2.diff removes padding on the .reorder-toggle class. Padding is already applied on the .reorder span contained inside the toggle, and this now gives consistency between the padding of this element and the corresponding reorder button in the Menu Customizer.

[obenland]

I think I'd rather change .reorder-toggle for widgets to use a button like in Menus. @TomHarrigan, would you mind refreshing your patch and see how big of a change it would be?

[TomHarrigan]

Replying to obenland:

I think I'd rather change .reorder-toggle for widgets to use a button like in Menus. @TomHarrigan, would you mind refreshing your patch and see how big of a change it would be?

Sure, @afercia mentioned that in #accessibility, will take a look at it during the hack night ;)

[obenland]

@TomHarrigan, have you had a chance to look into creating a new patch?

[TomHarrigan]

@obenland started on, got a lil caught up with work, will finish out and submit today if that works

[TomHarrigan]

Implement fix using button elements, aria tags to be consistent with Menu Customizer

[TomHarrigan]

Implement fix using button elements, aria tags to be consistent with Menu Customizer. Fix typo from 33260.3.diff

[afercia]

Notw that it's a button and has a type="button" attribute we should also review the events attached, needs just a click, no keys filtering and no preventDefault() :)

[obenland]

Replying to afercia:

Not that it's a button and has a type="button" attribute we should also review the events attached, needs just a click, no keys filtering and no preventDefault() :)

Do you have time to update the patch?

[TomHarrigan]

Replying to obenland:

Replying to afercia:

Not that it's a button and has a type="button" attribute we should also review the events attached, needs just a click, no keys filtering and no preventDefault() :)

Do you have time to update the patch?

type="button" also exists in the button in menu customizer content_template() function, so should it get removed there as well, and have those attached events reviewed too?

[obenland]

Replying to TomHarrigan:

type="button" also exists in the button in menu customizer content_template() function, so should it get removed there as well, and have those attached events reviewed too?

The Menu Customizer should be up to date on that.

[TomHarrigan]

Replying to obenland:

The Menu Customizer should be up to date on that.

The patch code was directly 'inspired' by the Menu Customizer code:

[obenland]

Oh, he meant that the JS events attached to .reorder-toggle in wp-admin/js/customize-widgets.php should be reviewed. Switching it to a button with type="button" allows us to remove the keyboard event checks.

[TomHarrigan]

Ah gotcha, I misread it as meaning that having button of type button was redundant

[afercia]

Replying to obenland:

Oh, he meant that the JS events attached to .reorder-toggle in wp-admin/js/customize-widgets.php should be reviewed. Switching it to a button with type="button" allows us to remove the keyboard event checks.

That :) adding type="button" removes any default action (in any browsers) from buttons. So there's no need to prevent a non-existing default action.

[afercia]

Refreshed patch to review events, dynamically update ARIA attributes and deliver wp.a11y.speak messages. This should pair these two buttons with the Menu Customizer buttons.
Removed moving focus to the first "Move to another area..." control for consistency with the Menu Customizer and also because that control is only visible when there are two or more widget areas.

[obenland]

We're in RC, so we should avoid introducing new strings at this point.
A refreshed patch without that would be pretty close to fixing the issue though.

[afercia]

Refreshed. If I'm not wrong, 'Reorder widgets' was the only one new, reverted to just 'Reorder' we should then change it for 4.4. The other 3 ones already exist.

[afercia]

Oh well I was wrong, there are also the strings added in the initial patches. What should we do with those ones? At this point maybe we should fix just the styling.

[helen]

Yes, at this point we should just fix the styling, and leave the rest for 4.4 (early).

[ocean90]

I agree, 33260.2.diff​ is OK to close this ticket for 4.3.

[obenland]

Thanks, will do

[obenland]

@helen, @ocean90, rather than removing it from widgets, what do you think about limiting menu styles to menus?

[ocean90]

33260.7.diff​ means, that we have to add .customize-control-sidebar_widgets to all three rules if we switch this to a <button> element.
But for 4.3 this works too. In 4.4 we should switch it to a <button> element and move it into customize-controls.css.

[obenland]

Replying to ocean90:

In 4.4 we should switch it to a <button> element and move it into customize-controls.css.

This.

[mrahmadawais]

@obenland @ocean90 I am up for this.
Actually, the real issue seems to be the .reorder-toggle padding in customize-widgets(-rtl).css file which is adding wrong padding.

I removed those paddings by making them 0 and voila it is perfectly aligned with the other span button. Let me add the patch.

P.S. I missed one comment in the discussion last time. But looks like, since we are not opting to make this span element a button we might as well just make that padding's value 0 for 4.3 release.

[mrahmadawais]

Fixes the Reorder Button by addressing the extra padding in span element

[mrahmadawais]

Reorder button is now realigned.

[mrahmadawais]

Replying to obenland:

Replying to ocean90:

In 4.4 we should switch it to a <button> element and move it into customize-controls.css.

This.

That's true, it will make the overall workflow better.

Kindly, look at the patch I submitted, it addresses the padding issue for the time being.

[obenland]

Hi mrahmadawais, thanks for your patch!

TomHarrigan orriginally suggested it too in 33260.2.diff​, but it's not widgets that do something wrong here, the menu styles are not properly name-spaced. 33260.7.diff​ fixes that.

[helen]

33260.7.diff​ seems good for 4.3, let's open a new ticket for 4.4 if there isn't one already. Needs second committer review but looks good to me.

[azaozz]

33260.7.diff looks good here too.

[obenland]

Replying to helen:

let's open a new ticket for 4.4 if there isn't one already.

#33327

[obenland]

In 33604:

Customize: Limit styles for menu's reorder button to the nav_menu control.

This prevents it from bleeding into other controls, misaligning their buttons.
Selectors use a class now too, which is preferable over ids.

Fixes #33260.