-
-
Notifications
You must be signed in to change notification settings - Fork 9
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
CI for accessibility of HTML report #66
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #66 +/- ##
=======================================
Coverage 53.25% 53.25%
=======================================
Files 42 42
Lines 4792 4792
=======================================
Hits 2552 2552
Misses 2009 2009
Partials 231 231 ☔ View full report in Codecov by Sentry. |
Warning Rate limit exceeded@Halleck45 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 7 minutes and 54 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe changes in the pull request enhance both the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Navigation
User->>Navigation: Navigate to Dashboard
Navigation-->>User: Focus on Dashboard link
User->>Navigation: Navigate to Refactoring candidates
Navigation-->>User: Focus on Refactoring candidates link
User->>Navigation: Navigate to Compare
Navigation-->>User: Focus on Compare link
User->>Navigation: Navigate to Help
Navigation-->>User: Focus on Help link
Possibly related PRs
Suggested labels
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
src/Report/Html/templates/layout.html (3)
Line range hint
122-144
: Add proper accessibility attributes to social linksThe social media links in the footer have malformed
aria-label
attributes and lack proper SVG accessibility.Apply these changes:
<a - href="https://twitter.com/Halleck45" class="mx-2 text-gray-600 transition-colors duration-300 aria-label="Twitter"> + href="https://twitter.com/Halleck45" + class="mx-2 text-gray-600 transition-colors duration-300" + aria-label="Follow on Twitter"> <svg class="w-5 h-5 fill-current" xmlns="http://www.w3.org/2000/svg" width="16" height="16" fill="currentColor" viewBox="0 0 16 16"> + <title>Twitter</title> <path d="M12.6.75h2.454l-5.36 6.142L16 15.25h-4.937l-3.867-5.07-4.425 5.07H.316l5.733-6.57L0 .75h5.063l3.495 4.633L12.601.75Zm-.86 13.028h1.36L4.323 2.145H2.865z"/> </svg> </a> <a - href="https://www.linkedin.com/in/jean-fran%C3%A7ois-l%C3%A9pine-6a122726" class="mx-2 text-gray-600 transition-colors duration-300 aria-label="Facebook"> + href="https://www.linkedin.com/in/jean-fran%C3%A7ois-l%C3%A9pine-6a122726" + class="mx-2 text-gray-600 transition-colors duration-300" + aria-label="Connect on LinkedIn"> <svg class="w-5 h-5 fill-current" role="img" viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg"> <title>LinkedIn</title> <path d="M20.447 20.452h-3.554v-5.569c0-1.328-.027-3.037-1.852-3.037-1.853 0-2.136 1.445-2.136 2.939v5.667H9.351V9h3.414v1.561h.046c.477-.9 1.637-1.85 3.37-1.85 3.601 0 4.267 2.37 4.267 5.455v6.286zM5.337 7.433c-1.144 0-2.063-.926-2.063-2.065 0-1.138.92-2.063 2.063-2.063 1.14 0 2.064.925 2.064 2.063 0 1.139-.925 2.065-2.064 2.065zm1.782 13.019H3.555V9h3.564v11.452zM22.225 0H1.771C.792 0 0 .774 0 1.729v20.542C0 23.227.792 24 1.771 24h20.451C23.2 24 24 23.227 24 22.271V1.729C24 .774 23.2 0 22.222 0h.003z"/> </svg> </a> <a - href="https://github.com/Halleck45/ast-metrics/" class="mx-2 text-gray-600 transition-colors duration-300 aria-label="Github"> + href="https://github.com/Halleck45/ast-metrics/" + class="mx-2 text-gray-600 transition-colors duration-300" + aria-label="View project on GitHub"> <svg class="w-5 h-5 fill-current" viewBox="0 0 24 24" fill="none" xmlns="http://www.w3.org/2000/svg"> + <title>GitHub</title> <path d="M12.026 2C7.13295 1.99937 2.96183 5.54799 2.17842 10.3779C1.395 15.2079 4.23061 19.893 8.87302 21.439C9.37302 21.529 9.55202 21.222 9.55202 20.958C9.55202 20.721 9.54402 20.093 9.54102 19.258C6.76602 19.858 6.18002 17.92 6.18002 17.92C5.99733 17.317 5.60459 16.7993 5.07302 16.461C4.17302 15.842 5.14202 15.856 5.14202 15.856C5.78269 15.9438 6.34657 16.3235 6.66902 16.884C6.94195 17.3803 7.40177 17.747 7.94632 17.9026C8.49087 18.0583 9.07503 17.99 9.56902 17.713C9.61544 17.207 9.84055 16.7341 10.204 16.379C7.99002 16.128 5.66202 15.272 5.66202 11.449C5.64973 10.4602 6.01691 9.5043 6.68802 8.778C6.38437 7.91731 6.42013 6.97325 6.78802 6.138C6.78802 6.138 7.62502 5.869 9.53002 7.159C11.1639 6.71101 12.8882 6.71101 14.522 7.159C16.428 5.868 17.264 6.138 17.264 6.138C17.6336 6.97286 17.6694 7.91757 17.364 8.778C18.0376 9.50423 18.4045 10.4626 18.388 11.453C18.388 15.286 16.058 16.128 13.836 16.375C14.3153 16.8651 14.5612 17.5373 14.511 18.221C14.511 19.555 14.499 20.631 14.499 20.958C14.499 21.225 14.677 21.535 15.186 21.437C19.8265 19.8884 22.6591 15.203 21.874 10.3743C21.089 5.54565 16.9181 1.99888 12.026 2Z"> </path> </svg> </a>
Line range hint
1-2
: Add language attribute and skip link for better accessibilityThe HTML document is missing crucial accessibility features:
- Language attribute for screen readers
- Skip link to help keyboard users bypass navigation
Add these improvements:
<!doctype html> -<html> +<html lang="en"> <head> + <style> + .skip-link { + position: absolute; + top: -40px; + left: 0; + background: #000; + color: white; + padding: 8px; + z-index: 100; + } + .skip-link:focus { + top: 0; + } + </style> </head> <body> + <a href="#main-content" class="skip-link">Skip to main content</a> <div class="grid grid-cols-7">Then add the corresponding ID to the main content:
- <main class="col-span-6"> + <main id="main-content" class="col-span-6">
Line range hint
92-119
: Improve share button accessibilityThe share functionality lacks proper accessibility features and keyboard support.
Apply these improvements:
- <div id="share-container" class="w-16 absolute right-10 grid grid-cols-2 gap-0"> + <div + id="share-container" + class="w-16 absolute right-10 grid grid-cols-2 gap-0" + role="group" + aria-label="Share options"> <div class="rounded px-1 py-1 hover:bg-gray-200"> <a + role="button" + aria-label="View project on GitHub" href="https://github.com/Halleck45/ast-metrics/"> <svg viewBox="0 0 24 24" aria-hidden="true" class="h-6 w-6 fill-slate-900"> <title>GitHub</title> <path fill-rule="evenodd" clip-rule="evenodd" d="M12 2C6.477 2 2 6.463 2 11.97c0 4.404 2.865 8.14 6.839 9.458.5.092.682-.216.682-.48 0-.236-.008-.864-.013-1.695-2.782.602-3.369-1.337-3.369-1.337-.454-1.151-1.11-1.458-1.11-1.458-.908-.618.069-.606.069-.606 1.003.07 1.531 1.027 1.531 1.027.892 1.524 2.341 1.084 2.91.828.092-.643.35-1.083.636-1.332-2.22-.251-4.555-1.107-4.555-4.927 0-1.088.39-1.979 1.029-2.675-.103-.252-.446-1.266.098-2.638 0 0 .84-.268 2.75 1.022A9.607 9.607 0 0 1 12 6.82c.85.004 1.705.114 2.504.336 1.909-1.29 2.747-1.022 2.747-1.022.546 1.372.202 2.386.1 2.638.64.696 1.028 1.587 1.028 2.675 0 3.83-2.339 4.673-4.566 4.92.359.307.678.915.678 1.846 0 1.332-.012 2.407-.012 2.734 0 .267.18.577.688.48 3.97-1.32 6.833-5.054 6.833-9.458C22 6.463 17.522 2 12 2Z"></path></svg> </a> </div> <div class="rounded px-1 py-1 hover:bg-gray-200"> - <a id="share-twitter" class="cursor-pointer"> + <button + id="share-twitter" + class="cursor-pointer" + aria-label="Share on Twitter" + role="button"> <svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" version="1.1" id="Layer_1" width="22px" height="22px" viewBox="0 0 22 22" style="enable-background:new 0 0 24 24;" xml:space="preserve"> + <title>Twitter</title> <path d="M14.095479,10.316482L22.286354,1h-1.940718l-7.115352,8.087682L7.551414,1H1l8.589488,12.231093L1,23h1.940717 l7.509372-8.542861L16.448587,23H23L14.095479,10.316482z M11.436522,13.338465l-0.871624-1.218704l-6.924311-9.68815h2.981339 l5.58978,7.82155l0.867949,1.218704l7.26506,10.166271h-2.981339L11.436522,13.338465z"/></svg> - </a> + </button> </div> </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
.github/workflows/test.yml
is excluded by!**/*.yml
lighthouserc.json
is excluded by!**/*.json
📒 Files selected for processing (1)
src/Report/Html/templates/layout.html
(4 hunks)
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.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
src/Report/Html/templates/index.html (1)
Line range hint
13-38
: Enhance tab navigation accessibilityThe language tabs section could benefit from additional ARIA attributes to improve screen reader support.
Consider these accessibility enhancements:
<!-- start: language tabs--> - <div class="flex overflow-x-auto overflow-y-hidden border-b border-gray-200 whitespace-nowrap "> + <div role="tablist" class="flex overflow-x-auto overflow-y-hidden border-b border-gray-200 whitespace-nowrap "> <a tabindex="0" href="index.html" + role="tab" + aria-selected="{% if currentLanguage == 'All' %}true{% else %}false{% endif %}" class="inline-flex items-center h-10 px-2 py-2 -mb-px text-center {% if currentLanguage == "All" %}text-blue-600 border-blue-500{% endif %} bg-transparent border-b-2 sm:px-4 -px-1 focus:outline-none"> <!-- ... --> </a> {% for languageName, lang in projectAggregated.ByProgrammingLanguage %} <a tabindex="0" href="index_{{ languageName }}.html" + role="tab" + aria-selected="{% if currentLanguage == languageName %}true{% else %}false{% endif %}" class="inline-flex items-center h-10 px-2 py-2 -mb-px text-center bg-transparent border-b-2 {% if currentLanguage == languageName %}text-blue-600 border-blue-500{% else %}text-gray-700 border-transparent{% endif %} sm:px-4 -px-1 hover:border-gray-400"> <!-- ... --> </a> {% endfor %}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
lighthouserc.json
is excluded by!**/*.json
📒 Files selected for processing (1)
src/Report/Html/templates/index.html
(2 hunks)
src/Report/Html/templates/index.html
Outdated
@@ -28,6 +29,7 @@ | |||
<!-- start: language tab --> | |||
{% for languageName, lang in projectAggregated.ByProgrammingLanguage %} | |||
<a | |||
tabindex="{{ 4 + forloop.index }}" |
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.
Avoid using positive tabindex values
Using positive tabindex
values (>0) is discouraged by accessibility best practices as it creates a custom tab order that might not match the visual/logical flow of the page. This can be confusing for keyboard users and screen readers.
Consider this alternative:
- tabindex="{{ 4 + forloop.index }}"
+ tabindex="0"
The natural DOM order of elements should dictate the tab sequence. If you need to change the tab order, consider restructuring the HTML instead.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
tabindex="{{ 4 + forloop.index }}" | |
tabindex="0" |
@@ -14,6 +14,7 @@ | |||
<div class="flex overflow-x-auto overflow-y-hidden border-b border-gray-200 whitespace-nowrap "> | |||
|
|||
<a | |||
tabindex="-1" |
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.
Reconsider using tabindex="-1" for the "All languages" link
Setting tabindex="-1"
removes this element from the natural tab order, which may harm accessibility. As this is the primary language selection option, users should be able to navigate to it using the Tab key.
Consider one of these alternatives:
- tabindex="-1"
+ tabindex="0"
or simply remove the tabindex attribute to keep it in the natural tab order:
- tabindex="-1"
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
tabindex="-1" |
Summary by CodeRabbit
tabindex
attributes to key links in the navigation menu, improving accessibility for users.tabindex
values for better control of tab order.