Problem/Motivation

The original problem described in issue #448292: Drag and Drop for table rows is not accessible to screen-reader users was described as follows: Users who are unable to operate the mouse, will not be able to use drag-drop-functionality. This was a release issue for D7 and accessibility. A patch was created for common, block and menu modules but it was pointed out in #280 that:

The patch adds #title and #title_display to the weight columns of these three modules, but there are many other places in core that use tabledrag (book admin, user role admin, profile admin, shortcut admin, image admin...) Shouldn't we be adding these labels everywhere? I'm not totally sure that has to be done in this issue or not, but it just seemed a little strange to me that this patch adds it some places but not others.

In #300, yoroy suggested that a separate issue be setup for implementing "everywhere". That is the purpose of this issue.

Proposed resolution

Add #title and #title_display to the weight column in files listed here:

  • modules/field/field.form.inc
  • modules/field_ui/field_ui.admin.inc
  • modules/book/book.admin.inc
  • modules/file/file.field.inc
  • modules/filter/filter.admin.inc
  • modules/image/image.admin.inc
  • modules/locale/locale.admin.inc
  • modules/profile/profile.admin.inc
  • modules/user/user.admin.inc

Remaining tasks

Patch has been written for d8 but has been failing with the following message: "Undefined index: label & filename aren't defined it seems." Errors need to be resolved

User interface changes

There will be an additional link "Show row weights" that will add the row weight column to the display for additional core module drag-and-drop tables.

Original report by [mgifford]

Following on @yoroy's point in #300 & my list of tabledrag elements in #296 I'm setting up a new issue so that we can discuss adding title & title display elements to the weight columns in the table drag elements.

To see the older thread see:
#448292: Drag and Drop for table rows is not accessible to screen-reader users

Comments

mgifford’s picture

Issue tags: +Accessibility

adding accessibility tag

frank ralf’s picture

Thanks, Mike!

Just for good measure I also asked my text editor for a list of all files containing the word "tabledrag", might be useful.

Search "tabledrag" (40 hits in 13 files)

\includes\common.inc (13 hits)
\modules\book\book.admin.inc (3 hits)
\modules\field\field.form.inc (1 hits)
\modules\field_ui\field_ui.admin.inc (4 hits)
\modules\file\file.field.inc (1 hits)
\modules\filter\filter.admin.inc (4 hits)
\modules\image\image.admin.inc (1 hits)
\modules\locale\locale.admin.inc (2 hits)
\modules\menu\menu.admin.inc (3 hits)
\modules\profile\profile.admin.inc (2 hits)
\modules\shortcut\shortcut.admin.inc (2 hits)
\modules\taxonomy\taxonomy.admin.inc (3 hits)
\modules\user\user.admin.inc (1 hits)

I'm not sure this captures all relevant instances, though.

David_Rothstein’s picture

Subscribing.

mgifford’s picture

Status: Active » Needs review

this is going to take some time..

Here's another one for the book module. Believe it only appears when ordering the books like here:
http://drupal7.dev.openconcept.ca/admin/content/book/124

I'm expecting this patch should fail but that we'll learn from it and keep posting additions for the other drag/drop tables in core.

mgifford’s picture

StatusFileSize
new1.17 KB

I thought that was attached last night....

mgifford’s picture

StatusFileSize
new8.62 KB

Ok, I've applied changes to these modules:

modules/field/field.form.inc
modules/field_ui/field_ui.admin.inc
modules/book/book.admin.inc
modules/file/file.field.inc
modules/filter/filter.admin.inc
modules/image/image.admin.inc
modules/locale/locale.admin.inc
modules/profile/profile.admin.inc
modules/user/user.admin.inc

But really need help in finding where to test them. I think I may have hit all of them here (but I'm not sure):

http://drupal7.dev.openconcept.ca/admin/structure/types/manage/a11y-surv...
http://drupal7.dev.openconcept.ca/admin/config/people/accounts/fields
http://drupal7.dev.openconcept.ca/admin/content/book/124
http://drupal7.dev.openconcept.ca/admin/config/media/image-styles/edit/zz
http://drupal7.dev.openconcept.ca/admin/config/people/profile
http://drupal7.dev.openconcept.ca/node/125/edit
http://drupal7.dev.openconcept.ca/admin/config/content/formats

Status: Needs review » Needs work
Issue tags: -Accessibility

The last submitted patch, better_support_for_drag_drop-3.patch, failed testing.

frank ralf’s picture

Status: Needs work » Needs review

#6: better_support_for_drag_drop-3.patch queued for re-testing because only MySQL errors which couldn't be caused by this patch.

Status: Needs review » Needs work
Issue tags: +Accessibility

The last submitted patch, better_support_for_drag_drop-3.patch, failed testing.

mgifford’s picture

Status: Needs work » Needs review

I think that's an error in the bot's display. I thought this too in previous bugs, but am pretty sure that there are going to need to be a lot of tests rewritten to allow for the labels.

bowersox’s picture

I am interested in helping fix the automated tests.

mgifford’s picture

Priority: Normal » Major
Issue tags: +simpletests

This patch applies to quite a few places in core that use drag/drop. Because of that I'm bumping this priority up to major from Normal.

We need help with simple tests too.

bowersox’s picture

Issue tags: -simpletests, -Accessibility

Status: Needs review » Needs work
Issue tags: +simpletests, +Accessibility

The last submitted patch, better_support_for_drag_drop-3.patch, failed testing.

bowersox’s picture

Status: Needs work » Needs review
StatusFileSize
new8.64 KB

Attached is a re-roll to keep up with HEAD. I have not fixed any of the tests. I spent my whole evening trying to sort out which changes caused which tests to fail. It is very time-consuming with so much in one patch. It takes a few minutes to run any sizable group of tests in my dev environment, so each time I make a change I need to wait a few more minutes to re-test.

It's helpful to have an issue that summarizes a whole group of these. But a mega-patch with dozens of tests failing is really inefficient to work with. Next time I sit down to work on this, I'm going to break this up into smaller hunks.

Status: Needs review » Needs work
Issue tags: -simpletests, -Accessibility

The last submitted patch, better_support_for_drag_drop-4.patch, failed testing.

Everett Zufelt’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work
Issue tags: +simpletests, +Accessibility

The last submitted patch, better_support_for_drag_drop-4.patch, failed testing.

dalin’s picture

-function _book_admin_table_tree($tree, &$form) {
+ function _book_admin_table_tree($tree, &$form) {

Shouldn't be adding that space.

t('Weight for @row', array('@row' => $data['link']['title']))

Like we did in #885616: Labels missing from a number of locale admin forms
These should be more semantic. i.e. @title rather than @row.

-        '#type' => 'textfield',
+        '#type' => 'hidden',
         '#default_value' => $data['link']['plid'],
-        '#size' => 6,

Why this change? It doesn't seem to relate to the title of this issue.

Otherwise it just needs a re-roll to catch up to HEAD plus some fixed tests.

mgifford’s picture

Status: Needs work » Needs review
StatusFileSize
new7.26 KB

I've re-rolled it. Nixed the extra space before function _book_admin_table_tree()

Many of the titles have already been changed to include more semantic text. I haven't gone through and cleaned all of this up however, and examples of @row still exist.

The plid was hidden (and also in a previous patch) because you shouldn't see the parent ID when you are displaying weights. There is no way to select a parent ID easily in D7. Hopefully this changes in D8.

I haven't touched the tests.

Status: Needs review » Needs work

The last submitted patch, better_support_for_drag_drop-5.patch, failed testing.

quicksketch’s picture

Version: 7.x-dev » 8.x-dev

I'd like to see some movement on this one. It's properly categorized IMO, but we've got too many major issues to add new features in D8 currently (see http://drupal.org/node/1201874). This issue would need to be fixed in D8 and then back-ported (a good candidate for this).

mgifford’s picture

Status: Needs work » Needs review
Issue tags: +Needs backport to D7
StatusFileSize
new4.21 KB

Hopefully this can help move this issue ahead. I've tagged that this is a good candidate for backporting, but of course it depends on getting it into D8 first.

Thanks @quicksketch

Status: Needs review » Needs work

The last submitted patch, better_support_for_drag_drop-827906-23.patch, failed testing.

mgifford’s picture

I'm not sure how to deal with these PHP notices from the test. Undefined index: label & filename aren't defined it seems. Not sure where the logic changed or if it's just a matter of patching further upstream.

Thoughts would be appreciated.

Everett Zufelt’s picture

tag

lucidus_neil’s picture

Added Issue Summary to ticket and removing Issue summary initiative tag.

mgifford’s picture

Status: Needs work » Closed (fixed)

I looked through the patch in #23 & think all of the title elements got in. I think we can close this issue.

mgifford’s picture

Issue summary: View changes

Added issue summary