feat(site): enhance HTML with semantic elements and ARIA accessibility#37
Merged
feat(site): enhance HTML with semantic elements and ARIA accessibility#37
Conversation
Contributor
|
Warning Rate limit exceeded
⌛ 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. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds pre-commit djlint hooks and two new validation scripts for embedded CSS and JS, updates pyproject with tinycss2 and py-mini-racer, and substantially rewrites the site HTML/JS to a multi-view layout with accessibility attributes, a manifest-builder modal, and new global modal/snippet/manifest functions. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Pre as Pre-commit
participant Hook as djlint Hooks
participant CSS as check_css.py
participant JS as check_js.py
participant HTML as Repository HTML Files
participant V8 as V8 (py-mini-racer)
Dev->>Pre: git commit / pre-commit run
Pre->>Hook: run djlint-check, check-embedded-css, check-embedded-js
Hook->>HTML: read HTML files
Hook->>CSS: invoke scripts/check_css.py (style extraction -> tinycss2)
CSS->>HTML: parse <style> blocks and validate
Hook->>JS: invoke scripts/check_js.py (script extraction -> V8)
JS->>V8: evaluate syntax in inline scripts
CSS-->>Hook: return diagnostics (ok|errors)
JS-->>Hook: return diagnostics (ok|errors)
Hook-->>Pre: aggregate exit code (0|1)
Pre-->>Dev: block or allow commit based on exit code
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
test/test_catalog_clib.py (1)
1-1:⚠️ Potential issue | 🟡 MinorRun
blackto fix formatting.The pipeline indicates this file needs reformatting. Run
black test/test_catalog_clib.pyto apply consistent formatting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/test_catalog_clib.py` at line 1, The test file test_catalog_clib.py is not formatted according to project standards; run the code formatter (black) on this file to fix formatting issues—e.g., execute `black test/test_catalog_clib.py` (or apply your repository's pre-commit formatter) and commit the resulting changes so the tests for dfetch_hub.catalog.clib (Packages.md parsing and limit handling) are properly formatted.test/test_catalog_conan.py (1)
1-1:⚠️ Potential issue | 🟡 MinorRun
blackto fix formatting.The pipeline indicates this file needs reformatting. Run
black test/test_catalog_conan.pyto apply consistent formatting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/test_catalog_conan.py` at line 1, The test file test_catalog_conan.py is misformatted; run the code formatter and commit the change by executing "black test/test_catalog_conan.py" (or your repo's pre-commit/format step) to apply consistent formatting to the module containing the Conan catalog tests (test/test_catalog_conan.py) and then stage and push the resulting formatted file.test/test_catalog_readme.py (1)
1-1:⚠️ Potential issue | 🟡 MinorRun
blackto fix formatting.The pipeline indicates this file needs reformatting. Run
black test/test_catalog_readme.pyto apply consistent formatting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/test_catalog_readme.py` at line 1, Run the code formatter "black" on the test file to fix formatting issues: apply black to test/test_catalog_readme.py (the module with the top docstring """Tests for dfetch_hub.catalog.sources.readme: README parsing and description extraction."""), e.g., run `black test/test_catalog_readme.py` and commit the resulting changes so the file conforms to project formatting rules.
🧹 Nitpick comments (1)
scripts/check_css.py (1)
27-28: Empty TYPE_CHECKING block can be removed.The
if TYPE_CHECKING: passblock serves no purpose. Consider removing lines 27-28.🧹 Remove empty TYPE_CHECKING block
-if TYPE_CHECKING: - pass -🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/check_css.py` around lines 27 - 28, Remove the no-op TYPE_CHECKING guard: delete the "if TYPE_CHECKING: pass" block and, if that makes the TYPE_CHECKING name unused, also remove its import from the top of the module so there are no unused imports; locate the empty block by the TYPE_CHECKING symbol in scripts/check_css.py (and adjust any related imports accordingly).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@dfetch_hub/site/index.html`:
- Around line 1175-1176: The _ESC map in index.html contains a malformed HTML
entity for '<' (currently '< ;' with an extra space) which breaks escaping;
fix the mapping in _ESC (referenced by esc(s)) by removing the stray space so
the value is '<' and ensure esc(s) continues to use _ESC for replacements.
In `@scripts/check_css.py`:
- Line 1: The file scripts/check_css.py is failing CI due to formatting; run the
project's formatter (black) on scripts/check_css.py and re-commit the changes to
satisfy the pipeline. If there are additional lint/format rules, run the same
command used by CI (e.g., black . or the repo's pre-commit hooks) to ensure
consistent formatting for functions like main or any validate_* helpers in that
file before pushing.
---
Outside diff comments:
In `@test/test_catalog_clib.py`:
- Line 1: The test file test_catalog_clib.py is not formatted according to
project standards; run the code formatter (black) on this file to fix formatting
issues—e.g., execute `black test/test_catalog_clib.py` (or apply your
repository's pre-commit formatter) and commit the resulting changes so the tests
for dfetch_hub.catalog.clib (Packages.md parsing and limit handling) are
properly formatted.
In `@test/test_catalog_conan.py`:
- Line 1: The test file test_catalog_conan.py is misformatted; run the code
formatter and commit the change by executing "black test/test_catalog_conan.py"
(or your repo's pre-commit/format step) to apply consistent formatting to the
module containing the Conan catalog tests (test/test_catalog_conan.py) and then
stage and push the resulting formatted file.
In `@test/test_catalog_readme.py`:
- Line 1: Run the code formatter "black" on the test file to fix formatting
issues: apply black to test/test_catalog_readme.py (the module with the top
docstring """Tests for dfetch_hub.catalog.sources.readme: README parsing and
description extraction."""), e.g., run `black test/test_catalog_readme.py` and
commit the resulting changes so the file conforms to project formatting rules.
---
Nitpick comments:
In `@scripts/check_css.py`:
- Around line 27-28: Remove the no-op TYPE_CHECKING guard: delete the "if
TYPE_CHECKING: pass" block and, if that makes the TYPE_CHECKING name unused,
also remove its import from the top of the module so there are no unused
imports; locate the empty block by the TYPE_CHECKING symbol in
scripts/check_css.py (and adjust any related imports accordingly).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2c585597-6864-458a-a0c7-3a3ad970a39d
📒 Files selected for processing (8)
.pre-commit-config.yamldfetch_hub/site/index.htmlpyproject.tomlscripts/check_css.pyscripts/check_js.pytest/test_catalog_clib.pytest/test_catalog_conan.pytest/test_catalog_readme.py
Addresses issue #28 by improving the semantic markup and accessibility of the single-page app while maintaining zero behavioral or visual changes. Semantic HTML5 improvements: - Topbar `<div>` converted to `<nav aria-label="External links">` - Hero `<div>` converted to `<section>` - Results list area `<main>` (nested incorrectly) converted to `<section>` - All view pages wrapped in a top-level `<main id="main-content">` - Breadcrumb `<div>` converted to `<nav aria-label="Breadcrumb">` - Heading hierarchy added: `<h1>` for hero title and package name, `<h2>` for sidebar filter groups and aside card titles - CSS heading reset added so `h1`–`h6` inherit font size/weight from their parent class styles ARIA accessibility improvements: - `aria-label` added to both search inputs, navbar, sort selects, manifest button, modal close button, and copy-snippet button - Tab bar: `role="tablist"`, `role="tab"`, `aria-selected`, and `aria-controls` on buttons; `role="tabpanel"` + `aria-labelledby` on panels; `switchTab()` JS updated to sync `aria-selected` - Snippet card header: `role="button"`, `tabindex="0"`, `aria-expanded`, and keyboard handler added; `toggleSnip()` and `renderDetail()` updated to keep `aria-expanded` in sync - Modal: `role="dialog"`, `aria-modal="true"`, `aria-labelledby` - Results count: `aria-live="polite"` and `aria-atomic="true"` - Toast notification: `role="status"` and `aria-live="polite"` - Decorative chevron spans marked `aria-hidden="true"` - Filter aside given `aria-label="Filter by"` - `aria-label` added to main navbar and package information tabs https://claude.ai/code/session_01LmU2X5jmgRZ1LHXc9S7bW1
spoorcc
force-pushed
the
claude/fix-issue-28-otA3r
branch
from
68d89ff to
2603ef0
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
scripts/check_js.py (1)
77-87: Return type annotation is overly generic.The return type
objectloses type information. Consider using a protocol orAnywith a clearer comment, or importMiniRacerinside aTYPE_CHECKINGblock for annotation purposes.♻️ Suggested improvement
+from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from py_mini_racer import MiniRacer -def _make_ctx() -> object: +def _make_ctx() -> "MiniRacer":🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/check_js.py` around lines 77 - 87, The return type for _make_ctx is currently object and should preserve the MiniRacer type: import TYPE_CHECKING from typing and inside an if TYPE_CHECKING block import MiniRacer from py_mini_racer, then change the signature of _make_ctx to return the MiniRacer type (or use Any from typing if you prefer a looser, no-runtime import). Update the annotation on def _make_ctx() to use the imported MiniRacer (or Any) so callers get proper type information while avoiding runtime import overhead.scripts/check_css.py (1)
28-29: Remove emptyTYPE_CHECKINGblock.The block contains only
passand serves no purpose.🧹 Remove dead code
-if TYPE_CHECKING: - pass🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/check_css.py` around lines 28 - 29, Remove the empty TYPE_CHECKING block in scripts/check_css.py: delete the "if TYPE_CHECKING: pass" stanza (and if the only use of TYPE_CHECKING is this block, also remove the corresponding import of TYPE_CHECKING) so no dead code remains; locate the snippet by searching for the "if TYPE_CHECKING" conditional in that file and remove it.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/check_css.py`:
- Around line 46-56: The stored style-block line is taken from getpos() in
handle_endtag, so errors point to the closing </style> instead of the opening
<style>; fix by capturing the start line in handle_starttag (call
self.getpos()[0] there and save to self._start_line when tag == "style"),
initialize self._start_line = 0 in __init__, and then in handle_endtag append
(self._start_line, "".join(self._buf)) to self.blocks instead of calling
getpos() there; keep the attrs parameter on handle_starttag (required by
HTMLParser) despite the linter warning.
- Around line 126-136: The code is parsing nested at-rule content with
tinycss2.parse_stylesheet(rule.content) but should use
tinycss2.parse_blocks_contents(rule.content) (or parse_rule_list() for old
compatibility) because rule.content is a list of ComponentValue tokens; replace
the parse_stylesheet call with parse_blocks_contents(rule.content) and keep the
subsequent loop and branches (checking inner_rule.type == "error" and calling
_check_declarations(inner_rule.content, start_line, block_num)) unchanged so
nested `@-rules` are parsed correctly.
In `@scripts/check_js.py`:
- Around line 52-64: The start line for script blocks is being captured at
handle_endtag via getpos(), which records the closing tag position; modify
handle_starttag to save the opening position (use self._start_line =
self.getpos()[0]) when entering a script (where you already set self._in_script
and self._buf), then change handle_endtag to append (self._start_line,
"".join(self._buf)) instead of calling getpos() there; also add an initializer
self._start_line: int = 0 in __init__ so the attribute exists.
---
Nitpick comments:
In `@scripts/check_css.py`:
- Around line 28-29: Remove the empty TYPE_CHECKING block in
scripts/check_css.py: delete the "if TYPE_CHECKING: pass" stanza (and if the
only use of TYPE_CHECKING is this block, also remove the corresponding import of
TYPE_CHECKING) so no dead code remains; locate the snippet by searching for the
"if TYPE_CHECKING" conditional in that file and remove it.
In `@scripts/check_js.py`:
- Around line 77-87: The return type for _make_ctx is currently object and
should preserve the MiniRacer type: import TYPE_CHECKING from typing and inside
an if TYPE_CHECKING block import MiniRacer from py_mini_racer, then change the
signature of _make_ctx to return the MiniRacer type (or use Any from typing if
you prefer a looser, no-runtime import). Update the annotation on def
_make_ctx() to use the imported MiniRacer (or Any) so callers get proper type
information while avoiding runtime import overhead.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 72b204c6-c344-4082-afb1-518b6b17b1c5
📒 Files selected for processing (5)
.pre-commit-config.yamldfetch_hub/site/index.htmlpyproject.tomlscripts/check_css.pyscripts/check_js.py
🚧 Files skipped from review as they are similar to previous changes (1)
- pyproject.toml
Three new pre-commit hooks (all PyPI-installable, language: python):
djlint-check — HTML formatter consistency
Runs djlint --check so any future hand-edits that deviate from djlint's
canonical HTML formatting are caught before commit.
Configuration additions in pyproject.toml:
profile = "html" — enables the full HTML5 rule set
indent = 2 — enforces 2-space indentation
max_line_length = 120 — matches the Python project limit
format_js = false — prevents djlint from mangling JS regex tokens
format_css = false — CSS is intentionally minified; keep it verbatim
check-embedded-css (scripts/check_css.py + tinycss2==1.5.1)
Extracts every <style> block from HTML files and parses each with
tinycss2. Reports parse errors at the tokeniser level (malformed tokens,
unexpected characters) and declaration-level errors (missing ':' separator,
empty value lists). Accepts modern CSS (custom properties, var(), clamp(),
vendor prefixes) without false positives.
check-embedded-js (scripts/check_js.py + py-mini-racer==0.6.0)
Extracts every inline <script> block (skipping src= external scripts) and
passes each through V8's Function constructor via py-mini-racer. This
catches all ES2020+ syntax errors (including optional chaining, async/await,
nullish coalescing) without executing the code.
Supporting changes:
• esc() refactored from 5 sequential .replace() calls to a single-pass
lookup-table approach (avoids djlint misreading the bare />/g regex token;
also slightly faster at runtime).
• index.html reformatted by djlint --reformat (HTML structure only; CSS/JS
content preserved verbatim).
• tinycss2==1.5.1 and py-mini-racer==0.6.0 added to [project.optional-
dependencies].development in pyproject.toml.
https://claude.ai/code/session_01LmU2X5jmgRZ1LHXc9S7bW1
spoorcc
force-pushed
the
claude/fix-issue-28-otA3r
branch
from
2603ef0 to
6db290e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses issue #28 by improving the semantic markup and accessibility of
the single-page app while maintaining zero behavioral or visual changes.
Semantic HTML5 improvements:
<div>converted to<nav aria-label="External links"><div>converted to<section><main>(nested incorrectly) converted to<section><main id="main-content"><div>converted to<nav aria-label="Breadcrumb"><h1>for hero title and package name,<h2>for sidebar filter groups and aside card titlesh1–h6inherit font size/weight fromtheir parent class styles
ARIA accessibility improvements:
aria-labeladded to both search inputs, navbar, sort selects,manifest button, modal close button, and copy-snippet button
role="tablist",role="tab",aria-selected, andaria-controlson buttons;role="tabpanel"+aria-labelledbyon panels;
switchTab()JS updated to syncaria-selectedrole="button",tabindex="0",aria-expanded, and keyboard handler added;toggleSnip()andrenderDetail()updated to keeparia-expandedin syncrole="dialog",aria-modal="true",aria-labelledbyaria-live="polite"andaria-atomic="true"role="status"andaria-live="polite"aria-hidden="true"aria-label="Filter by"aria-labeladded to main navbar and package information tabshttps://claude.ai/code/session_01LmU2X5jmgRZ1LHXc9S7bW1
Summary by CodeRabbit
New Features
Improvements