-
Notifications
You must be signed in to change notification settings - Fork 108
[MBL-19238][Teacher] Improve SpeedGrader slider accessibility #3469
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Summary
This PR enhances accessibility for the SpeedGrader grading interface by adding semantic descriptions to sliders and text fields for both percentage and points-based grading. The changes improve screen reader support and overall accessibility compliance.
Positive Aspects
✅ Good addition of accessibility string resources with proper formatting
✅ Consistent approach applied to both percentage and points grading types
✅ Proper use of clearAndSetSemantics to provide custom accessibility descriptions
✅ Dynamic content descriptions that reflect current grade values
✅ The string resource refactoring in DiscussionDetailsFragment.kt is clean and appropriate
Issues Found
- Potential precision loss in percentage calculation (line 701): Using
toIntOrNull()may truncate decimal values. Verify if decimal percentages should be supported. - Questionable
focused = falsesemantic (lines 710, 857): Setting focus state explicitly may interfere with natural focus management by accessibility services. - No-op
onClickhandler (lines 711, 857): The onClick action returnstruewithout performing any action, which may confuse accessibility services about what action is available.
Recommendations
-
Review the semantic modifiers: The
focused = falseand emptyonClickhandler pattern may not follow Compose accessibility best practices. Consider whether these are necessary or if the content description alone is sufficient. -
Test with TalkBack: Ensure the changes work well with Android's TalkBack screen reader, particularly the interaction between the custom semantics and the underlying text field behavior.
-
Consider decimal support: Verify whether the grading system should support decimal percentages (e.g., 95.5%) and adjust the
toIntOrNull()logic if needed.
Testing Suggestions
- Test with TalkBack enabled to verify the announcements are clear and accurate
- Verify that focus management works correctly when navigating between grading fields
- Test edge cases like empty fields, very large numbers, and decimal values
...va/com/instructure/pandautils/features/speedgrader/grade/grading/SpeedGraderGradingScreen.kt
Show resolved
Hide resolved
...va/com/instructure/pandautils/features/speedgrader/grade/grading/SpeedGraderGradingScreen.kt
Outdated
Show resolved
Hide resolved
...va/com/instructure/pandautils/features/speedgrader/grade/grading/SpeedGraderGradingScreen.kt
Outdated
Show resolved
Hide resolved
...va/com/instructure/pandautils/features/speedgrader/grade/grading/SpeedGraderGradingScreen.kt
Outdated
Show resolved
Hide resolved
📊 Code Coverage Reportℹ️ Student
ℹ️ Teacher
ℹ️ Pandautils
|
🧪 Unit Test Results✅ 📱 Teacher App
✅ 📦 Submodules
📊 Summary
Last updated: Mon, 12 Jan 2026 10:36:49 GMT |
kdeakinstructure
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
QA 👍
Test plan:
refs: MBL-19238
affects: Teacher
release note: Improved accessibility for SpeedGrader grading slider - screen readers now announce grade values clearly without confusing duplicate announcements
🤖 Generated with Claude Code
Co-Authored-By: Claude noreply@anthropic.com