Add tools section to Boost library views and templates#2049
Conversation
596fdda to
62db6a1
Compare
gregjkal
left a comment
There was a problem hiding this comment.
Looking good - couple of questions/suggested changes.
| "slug": "build", | ||
| "description": "The Boost build system, including the full Boost version of the jam sources.", | ||
| "version_specific": True, | ||
| "url_path": "tools/build/doc/html/index.html", |
There was a problem hiding this comment.
Have you tested these urls? I would think they should be root-relative, i.e. have a leading slash.
| "url_path": "tools/build/doc/html/index.html", | |
| "url_path": "/tools/build/doc/html/index.html", |
- Introduced a new TOOLS constant in constants.py containing various Boost tools with descriptions and URLs. - Implemented a get_tools function in utils.py to retrieve and sort the tools. - Updated LibraryListBase to include tools in the context data for library views. - Enhanced categorized_list.html, grid_list.html, and vertical_list.html templates to display tools alongside libraries, with appropriate descriptions and formatting.
- Added a new tool "boostlook" to the TOOLS constant in constants.py with its description and URL. - Modified the get_tools function in utils.py to accept an optional version parameter, allowing for dynamic URL generation for version-specific tools. - Updated LibraryListBase and LibraryCategorized views to pass the selected version to get_tools, ensuring correct URL handling in the context data.
- Updated _tool_categorized_list_item.html, _tool_grid_list_item.html, and _tool_vertical_list_item.html to conditionally render tool URLs, allowing for clickable rows and cards. - Simplified the display logic for tool names and descriptions, ensuring consistent styling and behavior across different list formats. - Enhanced accessibility by adding cursor pointer styles for clickable elements.
…to the Tools section. Includes tooltip on hover.
Co-authored-by: Greg Kaleka <greg@gregkaleka.com>
Co-authored-by: Greg Kaleka <greg@gregkaleka.com>
The version_slug hoist computed version.stripped_boost_url_slug before checking whether version was set, so calling get_tools() with no version threw AttributeError. The library list view does exactly that on some paths. Only compute the slug when a version is actually present.
0fa1603 to
5082283
Compare
📝 WalkthroughWalkthroughAdds a structured Boost tool catalog, resolves tool URLs by version, injects tools into library views, and renders them in categorized, grid, and vertical layouts with responsive link behavior. ChangesTool catalog integration
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant LibraryView
participant get_tools
participant Templates
LibraryView->>get_tools: request tools for selected version
get_tools-->>LibraryView: return sorted tools with resolved URLs
LibraryView->>Templates: provide tools in view context
Templates-->>LibraryView: render tool sections and items
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@templates/libraries/_tool_categorized_list_item.html`:
- Around line 1-7: Replace the URL-bearing clickable <tr> branch in the tool
list item with a normal link: render tool.name as an <a> using tool.url,
target="_blank", and rel="noopener noreferrer". Remove the inline onclick
behavior while preserving the existing non-link <tr> branch when tool.url is
empty.
In `@templates/libraries/_tool_vertical_list_item.html`:
- Around line 1-5: Update the tool.url branch of the row markup in
_tool_vertical_list_item.html to make the clickable <tr> focusable and
semantically interactive by adding appropriate tabindex and role attributes plus
a keyboard handler that activates the same window.open behavior for Enter and
Space. Apply the same accessibility fix to the corresponding clickable row in
_tool_categorized_list_item.html, while leaving rows without tool.url unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4190fc1f-30fe-4532-9577-a788e38fbb1c
📒 Files selected for processing (10)
libraries/constants.pylibraries/utils.pylibraries/views.pytemplates/libraries/_tool_categorized_list_item.htmltemplates/libraries/_tool_grid_list_item.htmltemplates/libraries/_tool_vertical_list_item.htmltemplates/libraries/categorized_list.htmltemplates/libraries/grid_list.htmltemplates/libraries/includes/library_preferences.htmltemplates/libraries/vertical_list.html
| {% if tool.url %} | ||
| <tr class="border-0 md:border border-gray-200/10 border-dotted md:border-t-0 md:border-r-0 md:border-l-0 md:border-b-1 hover:bg-gray-100 dark:hover:bg-gray-700 transition-all duration-200 ease-in-out cursor-pointer" onclick="window.open('{{ tool.url }}', '_blank', 'noopener,noreferrer')"> | ||
| {% else %} | ||
| <tr class="border-0 md:border border-gray-200/10 border-dotted md:border-t-0 md:border-r-0 md:border-l-0 md:border-b-1 hover:bg-gray-100 dark:hover:bg-gray-700 transition-all duration-200 ease-in-out"> | ||
| {% endif %} | ||
| <td class="py-2 align-top md:w-1/5"> | ||
| <span class="mr-1 font-bold capitalize text-sky-600 dark:text-sky-300">{{ tool.name }}</span> |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Use a real link for linked tools.
The clickable <tr> is not keyboard-accessible, provides no copyable/context-menu link, and interpolates tool.url into inline JavaScript. Render the tool name as an <a href> with target="_blank" and rel="noopener noreferrer" instead; keep the non-link branch for tools whose URL is empty.
🧰 Tools
🪛 HTMLHint (1.9.2)
[error] 1-1: Doctype must be declared before any non-comment content.
(doctype-first)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@templates/libraries/_tool_categorized_list_item.html` around lines 1 - 7,
Replace the URL-bearing clickable <tr> branch in the tool list item with a
normal link: render tool.name as an <a> using tool.url, target="_blank", and
rel="noopener noreferrer". Remove the inline onclick behavior while preserving
the existing non-link <tr> branch when tool.url is empty.
| {% if tool.url %} | ||
| <tr class="border-0 md:border border-gray-200/10 border-dotted md:border-t-0 md:border-r-0 md:border-l-0 md:border-b-1 hover:bg-gray-100 dark:hover:bg-gray-700 transition-all duration-300 ease-in-out cursor-pointer" onclick="window.open('{{ tool.url }}', '_blank', 'noopener,noreferrer')"> | ||
| {% else %} | ||
| <tr class="border-0 md:border border-gray-200/10 border-dotted md:border-t-0 md:border-r-0 md:border-l-0 md:border-b-1 hover:bg-gray-100 dark:hover:bg-gray-700 transition-all duration-300 ease-in-out"> | ||
| {% endif %} |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Tool rows with onclick are not keyboard-accessible.
When tool.url is set, the <tr> uses onclick to open the link but lacks tabindex, role, and a keyboard event handler. Keyboard-only users cannot focus or activate these rows, and there is no <a> fallback — this blocks task completion. The same pattern exists in _tool_categorized_list_item.html.
♿ Proposed fix: add keyboard support and ARIA semantics
{% if tool.url %}
- <tr class="border-0 md:border border-gray-200/10 border-dotted md:border-t-0 md:border-r-0 md:border-l-0 md:border-b-1 hover:bg-gray-100 dark:hover:bg-gray-700 transition-all duration-300 ease-in-out cursor-pointer" onclick="window.open('{{ tool.url }}', '_blank', 'noopener,noreferrer')">
+ <tr class="border-0 md:border border-gray-200/10 border-dotted md:border-t-0 md:border-r-0 md:border-l-0 md:border-b-1 hover:bg-gray-100 dark:hover:bg-gray-700 transition-all duration-300 ease-in-out cursor-pointer focus:outline focus:outline-2 focus:outline-sky-500" tabindex="0" role="link" onclick="window.open('{{ tool.url }}', '_blank', 'noopener,noreferrer')" onkeydown="if(event.key==='Enter'){window.open('{{ tool.url }}', '_blank', 'noopener,noreferrer')}">
{% else %}📝 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.
| {% if tool.url %} | |
| <tr class="border-0 md:border border-gray-200/10 border-dotted md:border-t-0 md:border-r-0 md:border-l-0 md:border-b-1 hover:bg-gray-100 dark:hover:bg-gray-700 transition-all duration-300 ease-in-out cursor-pointer" onclick="window.open('{{ tool.url }}', '_blank', 'noopener,noreferrer')"> | |
| {% else %} | |
| <tr class="border-0 md:border border-gray-200/10 border-dotted md:border-t-0 md:border-r-0 md:border-l-0 md:border-b-1 hover:bg-gray-100 dark:hover:bg-gray-700 transition-all duration-300 ease-in-out"> | |
| {% endif %} | |
| {% if tool.url %} | |
| <tr class="border-0 md:border border-gray-200/10 border-dotted md:border-t-0 md:border-r-0 md:border-l-0 md:border-b-1 hover:bg-gray-100 dark:hover:bg-gray-700 transition-all duration-300 ease-in-out cursor-pointer focus:outline focus:outline-2 focus:outline-sky-500" tabindex="0" role="link" onclick="window.open('{{ tool.url }}', '_blank', 'noopener,noreferrer')" onkeydown="if(event.key==='Enter'){window.open('{{ tool.url }}', '_blank', 'noopener,noreferrer')}"> | |
| {% else %} | |
| <tr class="border-0 md:border border-gray-200/10 border-dotted md:border-t-0 md:border-r-0 md:border-l-0 md:border-b-1 hover:bg-gray-100 dark:hover:bg-gray-700 transition-all duration-300 ease-in-out"> | |
| {% endif %} |
🧰 Tools
🪛 HTMLHint (1.9.2)
[error] 1-1: Doctype must be declared before any non-comment content.
(doctype-first)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@templates/libraries/_tool_vertical_list_item.html` around lines 1 - 5, Update
the tool.url branch of the row markup in _tool_vertical_list_item.html to make
the clickable <tr> focusable and semantically interactive by adding appropriate
tabindex and role attributes plus a keyboard handler that activates the same
window.open behavior for Enter and Space. Apply the same accessibility fix to
the corresponding clickable row in _tool_categorized_list_item.html, while
leaving rows without tool.url unchanged.
Summary by CodeRabbit
New Features
Style