WordPress.org

Make WordPress Core

Opened 5 years ago

Last modified 7 weeks ago

#23328 assigned enhancement

While editing widgets in accessibility mode the position can be indicated by widget titles instead of numbers

Reported by: mark-k Owned by: afercia
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Widgets Keywords: has-patch has-screenshots needs-refresh
Focuses: ui, accessibility, administration Cc:

Description

Guess it title says it all. Right now you need to figure out the position you want the widget to be at and try remember it while editing the widget.

I suggest to change the text text of the position column header from "position" to "position after" and use labels in the drop down

1 => "top"

2 => top widget title

3 => second widget title

etc

Attachments (5)

23328.patch (3.5 KB) - added by afercia 2 years ago.
23328.2.patch (3.6 KB) - added by afercia 19 months ago.
23328.3.patch (5.7 KB) - added by Mte90 19 months ago.
fix also #36772
23328.diff (5.8 KB) - added by afercia 14 months ago.
23328.2.diff (5.9 KB) - added by Mte90 13 months ago.

Download all attachments as: .zip

Change History (55)

#1 @_Redd
5 years ago

Hi mark-k, you may be interested in a related development here:

http://core.trac.wordpress.org/ticket/14045

Version 0, edited 5 years ago by _Redd (next)

#2 @chriscct7
3 years ago

  • Focuses accessibility administration added
  • Keywords needs-patch added

#3 @afercia
2 years ago

  • Focuses ui added

Not all users know this alternate mode so basically we're speaking about this, see in the screenshot below:

https://cldup.com/CRzCpWdnb2.png

This alternate screen can be enabled toggling the link "Enable/Disable accessibility mode" in the Screen Options. When reordering the widget position the select shows just numbers. It's difficult to remember the actual widgets position in each sidebar.

By the way, not all widgets have "titles" to show, several instances of the same widgets can be used on the sidebars, etc. Widgets do always have a widget type "name" though and we could at least add the widget type name. Additionally, the selects the edited widget is not assigned to, should have an empty, available, select option with some proper text, e.g. "Available position".

This screen could receive some UI tweaking too, for example I don't see any reason why the content should be centered, unless I'm missing something.

@afercia
2 years ago

#4 @afercia
2 years ago

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

A very fist pass, to get something like in the screenshot below. Any improvements and patches welcome :)

Couple of notes:

  • the option text should be shorter
  • noticed there's no "Cancel" button to cancel editing and go back to the Widgets screen

https://cldup.com/dj-OjRvZnS.png

This ticket was mentioned in Slack in #accessibility by rianrietveld. View the logs.


20 months ago

#6 @afercia
20 months ago

  • Milestone changed from Awaiting Review to 4.6
  • Owner set to afercia
  • Status changed from new to assigned

@afercia
19 months ago

#7 @afercia
19 months ago

Refreshed patch, coding standards, better variable names. Would appreciate a review.

#8 @afercia
19 months ago

Related: #36772. Needs to take care of the case of a widget from a disabled plugin that still has its "position" in the sidebar.

https://cldup.com/yIb2Qyfkud.png

@Mte90
19 months ago

fix also #36772

#9 @Mte90
19 months ago

  • Keywords dev-feedback added

Respect to the patch of @afercia contain:

  • In the select of the widget for the area if the widget not exist anymore (because remove like a plugin), that position is removed from the ui (added a new loop to add an empty option field)
  • On the save of the widgets of that sidebar now there is a check to detect if that widget is registered, in case it is removed from the db so in the next editing of that sidebar this problem don't happen again (like for the classic editing with drag & drop)

With this implementation fix also the #36772.

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


18 months ago

#11 @chriscct7
18 months ago

  • Milestone changed from 4.6 to Future Release

Punting per discussion

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


14 months ago

#13 @rianrietveld
14 months ago

  • Milestone changed from Future Release to 4.7

#14 @jorbin
14 months ago

  • Keywords needs-refresh added

The patch needs to be refreshed to follow the coding standards and also needs some additional testing with various AT.

#15 @afercia
14 months ago

In addition to some coding standards, there are some edge cases to check. While testing a bit, noticed there are cases where you can get more than one available empty position, see in the screenshot below:

https://cldup.com/QJBxNVcVSs.png

It seems to me this happens because some registered widgets have a hyphen in their name, for example recent-comments-1, recent-posts-1 so the patch fails when trying to get the widgets base id. I guess this is the reason why a few line below the base id is built first exploding in "pieces" the widget id and then re-adding the pieces together after removing the last piece.

Noticed also some widgets "disappearing" from their sidebar box, I guess because of the same issue, but I'd greatly appreciate some more eyes on the patch :)

@afercia
14 months ago

#16 @afercia
14 months ago

  • Keywords needs-refresh removed

Refreshed patch:

  • some coding standards
  • account for widgets IDs with a hyphen when getting the base ID

I'd really appreciate a couple more things here:

  • more meaningful variable names (though this could imply some refactoring)
  • a complete review :)

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


14 months ago

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


13 months ago

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


13 months ago

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


13 months ago

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


13 months ago

#22 @afercia
13 months ago

  • Keywords needs-refresh added; dev-feedback removed
  • Milestone changed from 4.7 to Future Release

Just tested again and this still needs to take into account the case of a widget from a disabled plugin that still has its "position" in the sidebar. To reproduce, activate a plugin that provides a widget, for example "Monster Widget" or the "Link Manager" plugin. Then add its widget to a sidebar, deactivate the plugin, and the sidebar will have an additional, empty, position available. Punting for now, if a refreshed patch comes to light in the next couple days please do feel free to move this back in the 4.7. milestone.

@Mte90
13 months ago

#23 @Mte90
13 months ago

  • Keywords needs-refresh removed

I added a little check to avoid double available position on the patch with 23328.2.diff

#24 follow-up: @afercia
13 months ago

Let's establish a test case:

  • put let's say 3 widgets in a sidebar
  • install https://wordpress.org/plugins/monster-widget/
  • go to the Widgets screen and enable the "accessibility mode"
  • activate the Monster Widget plugin and add its widget as 4th one in the sidebar
  • go back to Plugins and deactivate the Monster Widget plugin
  • go to the Widgets screen again and edit one of the 3 widgets
  • you will see there's a 4th available, empty, position that shouldn't be there

#25 @aaroncampbell
11 months ago

In 39760:

Add nonce for widget accessibility mode.

Props vortfu.

See #23328.

#26 @aaroncampbell
11 months ago

In 39761:

Add nonce for widget accessibility mode.

Props vortfu.

See #23328.

Merges [39760] to 4.7 branch.

#27 @aaroncampbell
11 months ago

In 39762:

Add nonce for widget accessibility mode.

Props vortfu.

See #23328.

Merges [39760] to 4.6 branch.

#28 @aaroncampbell
11 months ago

In 39763:

Add nonce for widget accessibility mode.

Props vortfu.

See #23328.

Merges [39760] to 4.5 branch.

#29 @aaroncampbell
11 months ago

In 39764:

Add nonce for widget accessibility mode.

Props vortfu.

See #23328.

Merges [39760] to 4.4 branch.

#30 @aaroncampbell
11 months ago

In 39765:

Add nonce for widget accessibility mode.

Props vortfu.

See #23328.

Merges [39760] to 4.3 branch.

#31 @aaroncampbell
11 months ago

In 39766:

Add nonce for widget accessibility mode.

Props vortfu.

See #23328.

Merges [39765] to 4.2 branch.

#32 @aaroncampbell
11 months ago

In 39767:

Add nonce for widget accessibility mode.

Props vortfu.

See #23328.

Merges [39765] to 4.1 branch.

#33 @aaroncampbell
11 months ago

In 39768:

Add nonce for widget accessibility mode.

Props vortfu.

See #23328.

Merges [39765] to 4.0 branch.

#34 @aaroncampbell
11 months ago

In 39769:

Add nonce for widget accessibility mode.

Props vortfu.

See #23328.

Merges [39765] to 3.9 branch.

#35 @aaroncampbell
11 months ago

In 39770:

Add nonce for widget accessibility mode.

Props vortfu.

See #23328.

Merges [39765] to 3.8 branch.

#36 @aaroncampbell
11 months ago

In 39771:

Add nonce for widget accessibility mode.

Props vortfu.

See #23328.

Merges [39765] to 3.7 branch.

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


4 months ago

#38 @afercia
4 months ago

  • Milestone changed from Future Release to 4.9

Let's try this (again) for the next release :)

#39 in reply to: ↑ 24 @Mte90
3 months ago

Replying to afercia:

Let's establish a test case:

  • put let's say 3 widgets in a sidebar
  • install https://wordpress.org/plugins/monster-widget/
  • go to the Widgets screen and enable the "accessibility mode"
  • activate the Monster Widget plugin and add its widget as 4th one in the sidebar
  • go back to Plugins and deactivate the Monster Widget plugin
  • go to the Widgets screen again and edit one of the 3 widgets
  • you will see there's a 4th available, empty, position that shouldn't be there

I tried right now and as I can see following this steps I see 4 (available position).
Looking on empty sidebar this is the right behaviour with an Number (available position).

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


2 months ago

#41 @afercia
2 months ago

this is the right behaviour

No, when there are 3 active widgets, there are 3 available positions. When adding a 4th widget, there are 4 available positions. After deactivating the Monster Widget, there should be just 3 available positions instead of 4.

However, I can't replicate this inconsistency on trunk any longer, while it can still be replicated on 4.8.2. I guess something was auto-magically fixed with the latest changes to the widgets, it would be nice to know _what_ changed :)

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


2 months ago

#43 @afercia
2 months ago

This change should be investigated a bit: https://core.trac.wordpress.org/changeset/41555

#44 @Mte90
2 months ago

Probably this lines do the magic:
`
Discard invalid, theme-specific widgets from sidebars.
$sidebars_widgets = _wp_remove_unregistered_widgets( $sidebars_widgets, $registered_widgets_ids );
$sidebars_widgets = wp_map_sidebars_widgets( $sidebars_widgets );
`

So probably this code actually is not necessary because somewhere there are new methods that clean up for wrong widgets.

This ticket was mentioned in Slack in #core-customize by afercia. View the logs.


2 months ago

#46 @afercia
8 weeks ago

  • Keywords needs-refresh added

I'm afraid after the recent changes in [41555] and [41594] the current patch doesn't work as expected. In some cases, I see one available position less than expected.

To my understanding, the recent changes to the widgets already take care of discarding invalid/orphaned widgets from the sidebars. Haven't checked in depth but I think now there's only the need to associate the available positions with the name of the widgets they're assigned to.

Note: At the time of writing this, there's only one week available for enhancements for 4.9 before Beta 1.

#47 @Mte90
8 weeks ago

I tried right now the patch and seems that is working (tried with a plugin and different widgets).
I am not sure what are the next steps, the (available position) need to became (available position for: {Widget name})?

#48 @afercia
8 weeks ago

@mte90 test a bit more please, I've just tried (again) and after I play a bit adding and removing widgets, here's what happens for me. Two active widgets in a sidebar:

https://cldup.com/ptNfYb_XEW.png

edit one of them and... position 2 should not be "available" but should be labeled with the Search widget:

https://cldup.com/t_6fhgc9v3.png

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


7 weeks ago

#50 @afercia
7 weeks ago

  • Milestone changed from 4.9 to Future Release

Beta 1 is in 2 days. Punting as per today's a11y bug scrub.

Note: See TracTickets for help on using tickets.