WordPress.org

Make WordPress Core

Opened 22 months ago

Last modified 66 minutes ago

#42001 assigned defect (bug)

Active Widget Not Fully Visible

Reported by: fervillz Owned by: audrasjb
Milestone: 5.3 Priority: normal
Severity: normal Version:
Component: Widgets Keywords: good-first-bug has-patch has-screenshots needs-refresh
Focuses: javascript Cc:

Description

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.

http://sheferstudio.com/wp-content/uploads/2017/09/wp-issue-active-widget.png

Attachments (9)

wp issue active widget.png (322.4 KB) - added by fervillz 22 months ago.
42001-v1.patch (916 bytes) - added by rinkuyadav999 22 months ago.
42001.patch (915 bytes) - added by jaydeep23290 22 months ago.
UI Issue.png (18.7 KB) - added by ashokrd2013 21 months ago.
Widget Issue.png (15.9 KB) - added by ashokrd2013 21 months ago.
42001.1.patch (1.1 KB) - added by ashokrd2013 21 months ago.
52dd9bb87ab970d65999cb8759341632.gif (403.9 KB) - added by audrasjb 3 months ago.
BEFORE widget text
14edcd8d766f2683409069feb243fdb6.gif (379.8 KB) - added by audrasjb 3 months ago.
AFTER widget text
2019-06-20_1200[1].png (114.0 KB) - added by ov1diu 4 weeks ago.
Active Widget Not Fully Visible Ticket ID#42001

Download all attachments as: .zip

Change History (32)

#1 @fervillz
22 months ago

  • Severity changed from normal to major

#2 @westonruter
22 months ago

  • Focuses javascript added
  • Keywords needs-patch good-first-bug added
  • Severity changed from major to normal
  • Version 4.8.2 deleted

#3 @rinkuyadav999
22 months ago

  • Keywords has-patch needs-testing added; needs-patch removed

#4 @ashokrd2013
21 months ago

  • Keywords needs-patch added; has-patch removed

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.


#5 @ashokrd2013
21 months ago

  • Keywords has-patch added; needs-patch removed

#6 @DrewAPicture
17 months ago

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

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

#7 @audrasjb
5 months ago

  • Keywords needs-testing removed
  • Milestone changed from Awaiting Review to 5.2

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.

#8 @audrasjb
4 months ago

  • Keywords commit added

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


3 months ago

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


3 months ago

@audrasjb
3 months ago

BEFORE widget text

@audrasjb
3 months ago

AFTER widget text

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


3 months ago

#12 @audrasjb
3 months ago

  • Keywords has-screenshots added

#13 @pento
3 months ago

  • Keywords needs-design-feedback added; commit removed

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.

This ticket was mentioned in Slack in #design by karmatosed. View the logs.


3 months ago

#15 follow-up: @melchoyce
3 months ago

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?

#16 in reply to: ↑ 15 @audrasjb
3 months ago

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 :-)

#17 follow-up: @pento
3 months ago

  • Keywords needs-refresh added; needs-design-feedback removed
  • Milestone changed from 5.2 to 5.3

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.

#18 in reply to: ↑ 17 @westonruter
3 months ago

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:

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

#19 @pento
3 months ago

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
4 weeks ago

Active Widget Not Fully Visible Ticket ID#42001

#20 @ov1diu
4 weeks ago

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

#21 @printsachen1
4 weeks ago

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

I tested also, and it works on 5.3

#22 @audrasjb
67 minutes ago

  • Resolution worksforme deleted
  • Status changed from closed to reopened

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

Reopening.

#23 @audrasjb
66 minutes ago

  • Owner changed from ashokrd2013 to audrasjb
  • Status changed from reopened to assigned
Note: See TracTickets for help on using tickets.