-
Notifications
You must be signed in to change notification settings - Fork 23
Accessibility Fixes for Indigo Theme/Plugin Based on Audit #34
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
Accessibility Fixes for Indigo Theme/Plugin Based on Audit #34
Conversation
128945a to
1bd5c10
Compare
ahmed-arb
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.
Thanks for this PR!
To help with the review, could you please:
- Provide "before and after" screenshots or a short video for the fixes/changes addressed in this PR.
- Summarize the specific changes made in this PR e.g "Improved button focus styling", "Adjusted 'Skip to Main Content' button positioning", etc.
This will greatly assist in verifying the fixes and ensure everything is working as expected.
paragon/_account.scss
Outdated
| .btn-brand, | ||
| .btn-primary, | ||
| .btn-tertiary, | ||
| .btn-outline-danger, | ||
| .btn-outline-primary, | ||
| .btn-inverse-outline-primary { | ||
| &:focus { | ||
| box-shadow: 0 0 0 2px #015fcc !important; | ||
| } | ||
| } |
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.
I am not sure, but general changes like these should be in generic files like _base.scss or _overrides.scss. So that we don't repeat them over and over, like we are doing in this PR.
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.
@ahmed-arb thanks for the review. I am going to remove this piece of code as rahat has made a generic fix in this PR which is also fixing this problem as well but another way.
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.
This code has been removed.
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's a github "No newline at the end of file" error here.
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.
done
@ahmed-arb , there are a lot of tickets addressed int this PR, so attaching screenshots for each cause PR messy, that's why attached taiga tickets links in the description, they have pretty details of the issue been addressed. |
|
@ahmed-arb I’ve updated the PR description to include a summary of the specific changes made and also added before/after screenshots for clarity. Let me know if there’s anything else you'd like me to adjust. |
@ahmed-arb schreenshot has been attached, please have a final look into it again. Thanks @arbirali |
Accessibility Fixes for Indigo Theme/Plugin
The audit team identified several accessibility-related issues within the Indigo Theme/Plugin. While investigating these issues, I discovered a package that required modifications to improve accessibility.
After a thorough review and debugging process, I implemented several fixes to address these problems. The issues were tracked on a Taiga board, and I’ve attempted to resolve all
brand-openedxrelated issues within a single pull request.🔧 Summary of Changes
I’ve included references to the corresponding issues below for additional context:
Related Issues on Taiga
🖼️ Screenshots/Sandbox
Please refer to the linked issues above for a deeper understanding of the context and the improvements made.
Related to overhangio/tutor-indigo#146