Closed (fixed)
Project:
Linkit
Version:
7.x-3.x-dev
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
4 Oct 2016 at 23:54 UTC
Updated:
22 Jan 2017 at 14:14 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
dsnopekUpdated IS to make it very explicit that this is an accessibility issue :-)
Comment #3
cboyden commentedI've attached a patch that adds an ID to the link/button that's added when Linkit is enabled for a field, then uses that ID to set focus when the dialog closes.
Comment #4
dsnopekThe patch works in my testing! However, I have some code review:
Explicitly supporting fields in linkit.js is not ideal.
Currently, field support is all abstracted away into linkit.field.js. It would be better to allow linkit.field.js to somehow affect the operation of modal, rather than have the modal know how linkit.field.js works. (Another way of thinking of this is that linkit.fields.js depends on linkit.js, whereas this code makes that into a circular dependency where they depend on each other.)
Maybe dialog helpers (like in linkit.field.js) could provide an 'onClose()' method which gets called? Or maybe Drupal.linkit.createModal() could take the 'activatingElement' as an argument that is focused on when the modal is closed?
linkit.field.js is currently referring to the button via the class "linkit-field-{$js_settings['source']}". It's weird to have this new code use this new ID, whereas the old code is using the class.
I think this patch should either use the class (like the existing code) or update the old code to use the ID too, so that we're not referring to the button in two different ways.
Comment #5
dsnopekHere's a new patch that implements my suggestions from #4. Please let me know what you think!
Comment #6
cboyden commentedLooks great, thanks @dsnopek. I took the liberty of removing some debug code, see attached interdiff and updated patch.
Comment #7
dsnopekWhoops! Thanks for catching that :-)
Comment #9
anonThanks for patches.