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
| Comment | File | Size | Author |
|---|---|---|---|
| #23 | better_support_for_drag_drop-827906-23.patch | 4.21 KB | mgifford |
| #20 | better_support_for_drag_drop-5.patch | 7.26 KB | mgifford |
| #15 | better_support_for_drag_drop-4.patch | 8.64 KB | bowersox |
| #6 | better_support_for_drag_drop-3.patch | 8.62 KB | mgifford |
| #5 | better_support_for_drag_drop-1.patch | 1.17 KB | mgifford |
Comments
Comment #1
mgiffordadding accessibility tag
Comment #2
frank ralf commentedThanks, Mike!
Just for good measure I also asked my text editor for a list of all files containing the word "tabledrag", might be useful.
I'm not sure this captures all relevant instances, though.
Comment #3
David_Rothstein commentedSubscribing.
Comment #4
mgiffordthis 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.
Comment #5
mgiffordI thought that was attached last night....
Comment #6
mgiffordOk, 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
Comment #8
frank ralf commented#6: better_support_for_drag_drop-3.patch queued for re-testing because only MySQL errors which couldn't be caused by this patch.
Comment #10
mgiffordI 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.
Comment #11
bowersox commentedI am interested in helping fix the automated tests.
Comment #12
mgiffordThis 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.
Comment #13
bowersox commented#6: better_support_for_drag_drop-3.patch queued for re-testing.
Comment #15
bowersox commentedAttached 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.
Comment #17
Everett Zufelt commented#15: better_support_for_drag_drop-4.patch queued for re-testing.
Comment #19
dalinShouldn'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.
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.
Comment #20
mgiffordI'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.
Comment #22
quicksketchI'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).
Comment #23
mgiffordHopefully 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
Comment #25
mgiffordI'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.
Comment #26
Everett Zufelt commentedtag
Comment #27
lucidus_neil commentedAdded Issue Summary to ticket and removing Issue summary initiative tag.
Comment #28
mgiffordI looked through the patch in #23 & think all of the title elements got in. I think we can close this issue.
Comment #28.0
mgiffordAdded issue summary