Make WordPress Core

Opened 11 years ago

Closed 5 years ago

Last modified 3 years ago

#23328 closed enhancement (wontfix)

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

Reported by: mark-k's profile mark-k Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Widgets Keywords: has-patch has-screenshots
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 8 years ago.
23328.2.patch (3.6 KB) - added by afercia 8 years ago.
23328.3.patch (5.7 KB) - added by Mte90 8 years ago.
fix also #36772
23328.diff (5.8 KB) - added by afercia 7 years ago.
23328.2.diff (5.9 KB) - added by Mte90 7 years ago.

Download all attachments as: .zip

Change History (59)

#1 @_Redd
11 years ago

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

Last edited 11 years ago by SergeyBiryukov (previous) (diff)

#2 @chriscct7
9 years ago

  • Focuses accessibility administration added
  • Keywords needs-patch added

#3 @afercia
8 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
8 years ago

#4 @afercia
8 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.


8 years ago

#6 @afercia
8 years ago

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

@afercia
8 years ago

#7 @afercia
8 years ago

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

#8 @afercia
8 years 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
8 years ago

fix also #36772

#9 @Mte90
8 years 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.


8 years ago

#11 @chriscct7
8 years 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.


8 years ago

#13 @rianrietveld
8 years ago

  • Milestone changed from Future Release to 4.7

#14 @jorbin
7 years 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
7 years 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
7 years ago

#16 @afercia
7 years 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.


7 years ago

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


7 years ago

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


7 years ago

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


7 years ago

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


7 years ago

#22 @afercia
7 years 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
7 years ago

#23 @Mte90
7 years 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
7 years 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
7 years ago

In 39760:

Add nonce for widget accessibility mode.

Props vortfu.

See #23328.

#26 @aaroncampbell
7 years ago

In 39761:

Add nonce for widget accessibility mode.

Props vortfu.

See #23328.

Merges [39760] to 4.7 branch.

#27 @aaroncampbell
7 years ago

In 39762:

Add nonce for widget accessibility mode.

Props vortfu.

See #23328.

Merges [39760] to 4.6 branch.

#28 @aaroncampbell
7 years ago

In 39763:

Add nonce for widget accessibility mode.

Props vortfu.

See #23328.

Merges [39760] to 4.5 branch.

#29 @aaroncampbell
7 years ago

In 39764:

Add nonce for widget accessibility mode.

Props vortfu.

See #23328.

Merges [39760] to 4.4 branch.

#30 @aaroncampbell
7 years ago

In 39765:

Add nonce for widget accessibility mode.

Props vortfu.

See #23328.

Merges [39760] to 4.3 branch.

#31 @aaroncampbell
7 years ago

In 39766:

Add nonce for widget accessibility mode.

Props vortfu.

See #23328.

Merges [39765] to 4.2 branch.

#32 @aaroncampbell
7 years ago

In 39767:

Add nonce for widget accessibility mode.

Props vortfu.

See #23328.

Merges [39765] to 4.1 branch.

#33 @aaroncampbell
7 years ago

In 39768:

Add nonce for widget accessibility mode.

Props vortfu.

See #23328.

Merges [39765] to 4.0 branch.

#34 @aaroncampbell
7 years ago

In 39769:

Add nonce for widget accessibility mode.

Props vortfu.

See #23328.

Merges [39765] to 3.9 branch.

#35 @aaroncampbell
7 years ago

In 39770:

Add nonce for widget accessibility mode.

Props vortfu.

See #23328.

Merges [39765] to 3.8 branch.

#36 @aaroncampbell
7 years 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.


7 years ago

#38 @afercia
7 years ago

  • Milestone changed from Future Release to 4.9

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

#39 in reply to: ↑ 24 @Mte90
7 years 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.


6 years ago

#41 @afercia
6 years 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.


6 years ago

#43 @afercia
6 years ago

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

#44 @Mte90
6 years 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.


6 years ago

#46 @afercia
6 years 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
6 years 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
6 years 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.


6 years ago

#50 @afercia
6 years ago

  • Milestone changed from 4.9 to Future Release

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

#51 @Mte90
6 years ago

Maybe a way to fix in all the situation is use _wp_remove_unregistered_widgets on uninstall a plugin or theme but this will not affect on deactive plugins.

#52 @afercia
5 years ago

  • Owner afercia deleted

#53 @afercia
5 years ago

  • Keywords needs-refresh removed
  • Milestone Future Release deleted
  • Resolution set to wontfix
  • Status changed from assigned to closed

As noted in the Gutenberg Phase 2 post, the wp-admin Widgets screen will be reworked to use blocks as Widgets. See https://github.com/WordPress/gutenberg/issues/13204

Hopefully, the new user interface will be designed with accessibility in mind from the beginning and finally WordPress will be able to remove the alternate "accessibility mode" used in the current Widgets screen. Serving the same (accessible) content to all users is always the best option.

I'm going to close this ticket as "wontfix". Sorry for the lack of a more proper keyword (a "replaced by" keyword would be more appropriate). Thanks everyone for the explorations and patches here. Tickets can always be reopened and discussion can continue on closed tickets.

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


3 years ago

Note: See TracTickets for help on using tickets.