Active Widget Not Fully Visible

[fervillz]

Patch to add "active" class to selected widget and have it fully visible. When you click a toggled/open widget, the widget should be on the front of the other widgets.

[ashokrd2013]

Below 2 files have parameter set for height & width, which needs to set 250 for height and width. It will display these 2 widget like other default widgets (Gallery, Tag etc..) and fix the issue related to design.

If size not made 250 then another issue also occurs see below screenshots.

src\wp-includes\widgets\class-wp-widget-custom-html.php
src\wp-includes\widgets\class-wp-widget-text.php

Below is the fix for all issues.

[DrewAPicture]

Assigning to mark the good-first-bug as "claimed".

[audrasjb]

Hi and thanks for the patch!

I tested the patch and it works fine on my side.
The patch still applies cleanly.

Moving to 5.2.

[audrasjb]

BEFORE widget text

[audrasjb]

AFTER widget text

[pento]

Thank you for the GIFs, @audrasjb. This is super helpful for seeing the effect of the change.

I'd like to get a design review on this before we go ahead with it.

[melchoyce]

Chatted about this in today's ​design triage, and we're generally in favor of this change. We have a couple questions:

• @pento Do you have any background on why widgets overflow the widget area containers in the first place?
• @audrasjb Does this patch impact the Customizer as well?

[audrasjb]

Replying to melchoyce:

• @audrasjb Does this patch impact the Customizer as well?

Nope, only the widgets screen.

In my opinion, this is a good candidate for 5.2 since it's pretty self contained.

We can also punt it to the next milestone but that wouldn’t make so much sense to me as this is a bug that can be easily fixed :-)

[pento]

Thanks for the review, @melchoyce!

I've been doing a little archeology through the codebase, and I have a couple of open questions:

• Why do we still support wide widgets? @shaunandrews, you were the last person to ​make a definitive comment on this that I could find.
• Why are HTML, Text, and RSS widgets wide?

Regarding the patch:

• We don't actually need to change the height, since it ​apparently isn't used.
• The RSS widget is also wide: if we're updating wide widgets, we should do that one, too.

I'm going to punt this issue to 5.3, I'd appreciate if folks could do some more investigation into this.

[westonruter]

Replying to pento:

• Why are HTML, Text, and RSS widgets wide?

I assume the Text widget is wide in order to give more room to edit. The Custom HTML widget is wide because the Text widget was wide, as I recall. I don't know about the RSS widget's reason.

The Text widget has been wide since it was introduced in 2.2 (#4169), as far as I can tell:

• ​https://github.com/WordPress/wordpress-develop/blob/7488c46fd14e8328029796e009743a0592382a8b/wp-includes/widgets.php#L455-L456
• ​https://github.com/WordPress/wordpress-develop/commit/bbdd020aa16e8cb139d74333896bf9b62a6b2c8b

The ​Widgets feature plugin that was merged apparently had wide Text widget, so that is why it persisted. The first feature plugin commit had a widget_text_register() which included the wide widget:

<?php
register_widget_control($name, $i <= $number ? 'widget_text_control' : /* unregister */ '', 460, 350, $i);

​https://plugins.trac.wordpress.org/browser/widgets/trunk/widgets.php?rev=5614#L792

[pento]

Yeah, it seems like the width of the widget column has changed over the years, both shrinking and growing. I couldn't find any reference when it shrunk it its current size as to why these widgets stayed wide.

Either way, we still support wide widgets, so this is a broader problem: we can make core widgets narrower, but custom widgets will still stuffer the problem originally reported.

[ov1diu]

Active Widget Not Fully Visible Ticket ID#42001

[ov1diu]

As seen above, the issue doesn't replicate anymore.
Ticket can close.
Thanks!

[printsachen1]

I tested also, and it works on 5.3

[audrasjb]

Please do not close tickets that are not committed into core @printsachen1

Reopening.

[audrasjb]

Reducing widgets width to 250

[audrasjb]

Fixed widget width for all wide widgets

[audrasjb]

Hi there,

In 42001.2.diff:

• reduce width (and only width) parameter to 250
• apply that new width to both Text, Custom HTML and RSS Widgets

See animated screenshot above.

[audrasjb]

Removing commit keyword to see how we can handle custom widgets as well.

[audrasjb]

No feedback on this one. Should we commit the current patch so the core widget will be ok or punt it to 5.4 to work on custom widgets as well. I'm all in favor to commit it as it. Any thoughts on that @pento @westonruter ?

[audrasjb]

The ticket is in my opinion very close to its resolution, but WP 5.3 is already in Beta 2, let's move this ticket to milestone 5.4 early so it could be handled in the next version.

[audrasjb]

Hi there,

I think we need to fix this issue at least for core widgets. Custom widgets should be addressed in another ticket.
The patch still applies cleanly, so let's tag it for commit.

[SergeyBiryukov]

As noted in comment:18, ever since the introduction of widgets in [5294] / #4169, the Text and RSS widgets have been wider than others to provide more space for input. Others generally just have a title input and a few checkboxes and don't require additional width.

The Custom HTML widget was added in [40893] / #40907. Being modeled after the Text widget, it kept the same width.

Some follow-up fixes that touch wide widgets one way or another over the years:

• [5294] / #4169
• [5301] / #4169
• [5399] / #4169
• [6556] / #5583
• [7086] / #5997
• [10912] / #9511
• [10916] / #9511
• [11160] / #9511
• [21281] / #21247
• [26277] / #26117

A related IRC discussion from #26117:
​https://irclogs.wordpress.org/chanlog.php?channel=wordpress-dev&day=2013-11-19&sort=asc#m732752

My concerns with 42001.2.diff​ (restricting all widgets to the sidebar width) are:

• Providing less space for input, making a narrow UI element even narrower, which doesn't seem ideal.
• It might fix the issue for core, but not for any custom widgets.
• Removing wide widgets from core and declaring that plugins or themes should no longer be use them seems like a regression, there could be other use cases for this additional width.
• In general, it feels more like a workaround than really addressing the issue.

Since the main issue here is that an active widget is not fully visible due to overlapping with another widget, I would suggest addressing that.

The widgets on this screen currently have the following z-index hierarchy:

• Wide widget, open (z-index: 100).
• Any widget being dragged from the Available Widgets area to a sidebar (z-index: 100).
• Regular widget, open (no z-index).
• Any widget, closed (no z-index).

We could utilize the ​:focus-within CSS pseudo-class (supported by ​all major browsers) to improve things a bit.

In 42001.4.diff​, the hierarchy is adjusted to make sure widget currently being interacted with always stays on top:

• Any widget being dragged from the Available Widgets area to a sidebar (z-index: 101).
• Any open widget with a current focus (z-index: 100)
• Any widget, open (z-index: 99).
• Any widget, closed (no z-index).

[audrasjb]

Wow, thanks for the detailed comment @SergeyBiryukov !
Currently testing the last proposal.

[audrasjb]

Works fine!

[audrasjb]

I tested 42001.4.diff and it works like a charm for all modern browsers.

Thanks @SergeyBiryukov for this perfect alternative! Adding commit keyword.

[SergeyBiryukov]

Replying to SergeyBiryukov:

As noted in comment:18, ever since the introduction of widgets in [5294] / #4169, the Text and RSS widgets have been wider than others to provide more space for input. Others generally just have a title input and a few checkboxes and don't require additional width.

The Custom HTML widget was added in [40893] / #40907. Being modeled after the Text widget, it kept the same width.

Related: #42081, specifically comment:2:ticket:42081.

[SergeyBiryukov]

In 47263:

Widgets: Prevent currently active wide widget controls from being partially covered by another widget's controls.

The currently active widget controls should always be displayed on top.

Props audrasjb, fervillz, rinkuyadav999, jaydeep23290, ashokrd2013, melchoyce, pento, westonruter, SergeyBiryukov.
Fixes #42001.