Allow dropdown inputs to be manually enabled for DnD questions for accessibility #2159
Allow dropdown inputs to be manually enabled for DnD questions for accessibility #2159sjd210 wants to merge 32 commits into
Conversation
This reverts commit 602c7f7.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2159 +/- ##
==========================================
+ Coverage 43.75% 43.81% +0.06%
==========================================
Files 593 594 +1
Lines 25036 25085 +49
Branches 7427 8330 +903
==========================================
+ Hits 10954 10992 +38
- Misses 14032 14044 +12
+ Partials 50 49 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…nput-setting [VRT] Update baselines for feature/non-dragging-input-setting
…nput-setting [VRT] Update baselines for feature/non-dragging-input-setting
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…nput-setting [VRT] Update baselines for feature/non-dragging-input-setting
jacbn
left a comment
jacbn
left a comment
There was a problem hiding this comment.
This is looking really good! I have one comment here about keeping hooks inside /services but beyond that my only consideration here is where and how we display the toggle.
As we've discussed, perhaps the most important thing for accessibility across the site is that we are consistent in how we surface accessibility features, so that using a new question type does not feel completely alien to e.g. a screenreader user.
Having a toggle at the top (which may be skipped if the user jumps to a specific section via a hash anchor) for just this question type does feel a little specific, though I can see it having value, particularly for tablets. To this end, I'm very happy for this feature to exist in the accessibility settings (and as you mention, it can remain on the page for non-logged in users), but if our plans for accessibility on other question types do not include such a toggle, I think I'd prefer we improve the accessibility on Cloze by allowing click-to-select and click-to-drop, so that we remain consistent in that all questions can be answered accessibly without requiring a toggle elsewhere on the page.
I'd love to be able to ask an accessibility panel about how they feel about which method is most accessible, as I can only base this off being consistent with other questions... :/
| }; | ||
|
|
||
| const IsaacDragAndDropQuestion = ({doc, questionId, readonly, validationResponse}: IsaacQuestionProps<IsaacDragAndDropQuestionDTO, DndValidationResponseDTO>) => { | ||
| export function useDragAndDropAccessibility() { |
There was a problem hiding this comment.
Since we use this on Cloze qs too, I'd move this hook out into /services/accessibility.ts (or a new dnd-specific one if you prefer) – this is where generally useful hooks or those that use actions/reducers/slices etc are supposed to live.
The 'why' is that we shouldn't really ever be importing question types inside other question types, for two reasons – one, all questions are lazily loaded, so it increases the page load time slightly if we need to import from another lazy file, and two, it avoids the possibility of circular dependencies.
Of course, none of this is to say that these things don't happen in the codebase. But we ought to stick to good style where we can!
4 participants
Adds an accessibility setting to "Enable non-dragging alternative inputs". For now this only affects cloze/drag-and-drop questions, but is defined broadly enough that we can use the same setting for other inputs in the future if they apply (e.g. depending on how we implement reorder question accessibility).
Also adds a
Dropdown/Drag and dropoverride toggle to all question and quiz pages containing these input types in the top right, equivalent to the context switcher on concept pages. The toggle is defaulted toDropdownif the user has enabled the accessibility setting or is on a mobile device, andDrag and dropotherwise. If manually toggled, this overrides all other settings and the toggled input mode is enabled instead (e.g. allowing drag and drop at xs screens, if wanted).I'm not 100% confident on things like:
Thoughts welcome!
Also requires https://github.com/isaacphysics/isaac-sops-config/pull/22 for new accessibility setting to work.Merged!Known problem: If a question is using dropdowns on first page load, then the first time drag and drop is switched to the dropzones will not load properly. This is a pre-existing issue (try some incremental page width changes on a live question) but is surfaced a lot more heavily here. This is sure to be some portalling shenanigans, and I don't think we should merge this without fixing it, but the rest is ready to review now.