WordPress.org

Make WordPress Core

Opened 2 years ago

Last modified 7 weeks ago

#42001 assigned defect (bug)

Active Widget Not Fully Visible

Reported by: fervillz Owned by: audrasjb
Milestone: 5.4 Priority: normal
Severity: normal Version:
Component: Widgets Keywords: has-patch has-screenshots needs-refresh early
Focuses: javascript Cc:
PR Number:

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 (11)

wp issue active widget.png (322.4 KB) - added by fervillz 2 years ago.
42001-v1.patch (916 bytes) - added by rinkuyadav999 2 years ago.
42001.patch (915 bytes) - added by jaydeep23290 2 years ago.
UI Issue.png (18.7 KB) - added by ashokrd2013 2 years ago.
Widget Issue.png (15.9 KB) - added by ashokrd2013 2 years ago.
42001.1.patch (1.1 KB) - added by ashokrd2013 2 years ago.
52dd9bb87ab970d65999cb8759341632.gif (403.9 KB) - added by audrasjb 7 months ago.
BEFORE widget text
14edcd8d766f2683409069feb243fdb6.gif (379.8 KB) - added by audrasjb 7 months ago.
AFTER widget text
2019-06-20_1200[1].png (114.0 KB) - added by ov1diu 5 months ago.
Active Widget Not Fully Visible Ticket ID#42001
42001.2.diff (1.6 KB) - added by audrasjb 4 months ago.
Reducing widgets width to 250
083bbff4d2b6fc341b4f85e94eb07622.gif (3.3 MB) - added by audrasjb 4 months ago.
Fixed widget width for all wide widgets

Change History (39)

#1 @fervillz
2 years ago

  • Severity changed from normal to major

#2 @westonruter
2 years ago

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

#3 @rinkuyadav999
2 years ago

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

@jaydeep23290
2 years ago

@ashokrd2013
2 years ago

#4 @ashokrd2013
2 years 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.


@ashokrd2013
2 years ago

#5 @ashokrd2013
2 years ago

  • Keywords has-patch added; needs-patch removed

#6 @DrewAPicture
22 months ago

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

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

#7 @audrasjb
9 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
8 months ago

  • Keywords commit added

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


8 months ago

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


7 months ago

@audrasjb
7 months ago

BEFORE widget text

@audrasjb
7 months ago

AFTER widget text

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


7 months ago

#12 @audrasjb
7 months ago

  • Keywords has-screenshots added

#13 @pento
7 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.


7 months ago

#15 follow-up: @melchoyce
7 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
7 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
7 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
7 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
7 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
5 months ago

Active Widget Not Fully Visible Ticket ID#42001

#20 @ov1diu
5 months ago

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

#21 @printsachen1
5 months ago

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

I tested also, and it works on 5.3

#22 @audrasjb
4 months 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
4 months ago

  • Owner changed from ashokrd2013 to audrasjb
  • Status changed from reopened to assigned

@audrasjb
4 months ago

Reducing widgets width to 250

@audrasjb
4 months ago

Fixed widget width for all wide widgets

#24 @audrasjb
4 months ago

  • Keywords commit added; needs-refresh removed

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.

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


3 months ago

#26 @audrasjb
3 months ago

  • Keywords commit removed

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

#27 @audrasjb
2 months ago

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 ?

#28 @audrasjb
7 weeks ago

  • Keywords needs-refresh early added; good-first-bug removed
  • Milestone changed from 5.3 to 5.4

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.

Note: See TracTickets for help on using tickets.