-
-
Notifications
You must be signed in to change notification settings - Fork 950
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
SAK-50757 Gradebook modals launched from cell not receiving focus #13125
base: 23.x
Are you sure you want to change the base?
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.
There are accessibility issues in these changes.
...ebookng/tool/src/java/org/sakaiproject/gradebookng/tool/panels/CourseGradeOverridePanel.html
Show resolved
Hide resolved
...okng/tool/src/java/org/sakaiproject/gradebookng/tool/panels/EditCourseGradeCommentPanel.html
Show resolved
Hide resolved
@@ -7,7 +7,7 @@ | |||
<form class="form-vertical" wicket:id="form"> | |||
<div wicket:id="editCommentFeedback"></div> | |||
|
|||
<textarea wicket:id="comment" id="gradeComment" rows="4" class="form-control awesomplete"></textarea> | |||
<textarea wicket:id="comment" id="gradeComment" rows="4" class="form-control awesomplete my-3"></textarea> |
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.
Looks like this element is missing an accessible name or label. That makes it hard for people using screen readers or voice control to use the control.
gradebookng/tool/src/java/org/sakaiproject/gradebookng/tool/panels/ZeroUngradedItemsPanel.html
Show resolved
Hide resolved
@hornersa Are contributions still active for this project what is the status of this issue is it resolved? |
Hi @samr874 this is a pull request waiting for review. are you table to test it and confirm it fixes the bug? |
@ottenhoff sure let me review this then will update you with required changes |
Hi @samr874 how did your testing go? |
// instead of navigating to the next tabbable control in the modal. | ||
event.stopImmediatePropagation(); | ||
return; | ||
} |
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.
what exactly is this doing?
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.
To answer that, I've posted an explainer in the form of a screencast attached to the corresponding jira, titled SAK-50757-ProposedFixExplainer-20250107.mp4.
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.
@adrianfish does this fix make sense to you for 23.x? it seems like your fix in SAK-49442 has modified/broken this modal behavior in 23.x
My testing shows that this is a likely improvement. It's not perfect as it doesn't trap the tabbing inside the modal the way it should (The way Bootstrap modal does). But it's still an improvement. |
Jira: https://sakaiproject.atlassian.net/browse/SAK-50757
The demo for this proposed fix plus some explanations regarding it are in a screencast posted to the jira: SAK-50757-ProposedFixDemo-20241219.mp4.