POC: Ask AI button and Kapa widget#108
Conversation
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
layouts/partials/footer.html (2)
110-110: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePotential ID collision between
ai-buttonsandfooter-buttons.Both the
ai-buttonstemplate (line 110:id="footer-btn-{{ lower .label }}") and thefooter-buttonstemplate (line 90:"idPrefix" "footer-btn-") use thefooter-btn-ID prefix. If any social button text lowercases to a provider label (e.g., "ChatGPT"), the IDs will collide. Consider using a distinct prefix for AI provider buttons, such asfooter-ai-btn-.♻️ Suggested fix
- <a class="gd-footer-buttons-item-icon-button" href="{{ printf "%s%s" .base (urlquery $query) | safeURL }}" id="footer-btn-{{ lower .label }}" aria-label="{{ .label }}" target="_blank" rel="noreferrer noopener"> + <a class="gd-footer-buttons-item-icon-button" href="{{ printf "%s%s" .base (urlquery $query) | safeURL }}" id="footer-ai-btn-{{ lower .label }}" aria-label="{{ .label }}" target="_blank" rel="noreferrer noopener">🤖 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 `@layouts/partials/footer.html` at line 110, Change the id prefix in the AI provider button anchor within the ai-buttons template from footer-btn- to a distinct prefix such as footer-ai-btn-. Keep the label-based suffix unchanged, and leave the footer-buttons IDs untouched.
99-124: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTrailing space inside the
aria-labelconditional inabsolute-link.Line 124 emits
aria-label="{{ .ariaLabel }}"with a trailing space before{{ end }}. This is harmless in HTML but produces a stray space attribute separator. Consider removing the trailing space for cleaner output.♻️ Suggested fix
- <a href="{{ $url }}" class="{{ .class }}" id="{{ .idPrefix }}{{ $id }}" target="_blank" {{ if (isset . "ariaLabel") }}aria-label="{{ .ariaLabel }}" {{ end }}{{ if (isset . "rel") }}rel="{{ .rel }}"{{end}}>{{ $content }}</a> + <a href="{{ $url }}" class="{{ .class }}" id="{{ .idPrefix }}{{ $id }}" target="_blank"{{ if (isset . "ariaLabel") }} aria-label="{{ .ariaLabel }}"{{ end }}{{ if (isset . "rel") }} rel="{{ .rel }}"{{ end }}>{{ $content }}</a>🤖 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 `@layouts/partials/footer.html` around lines 99 - 124, Remove the trailing whitespace emitted by the ariaLabel conditional in the absolute-link template, while preserving the conditional aria-label attribute and surrounding link attributes.
🤖 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 `@layouts/partials/ask-ai-button.html`:
- Line 12: Remove the static aria-controls="kapa-modal-content" attribute from
the ask-AI button markup. Keep the dynamic handling in ask-ai-accessibility.js
unchanged so it adds the attribute when the modal appears and removes it when
the modal disappears.
In `@layouts/partials/footer.html`:
- Line 17: Move the ai-buttons template invocation outside the bannerLinks range
loop, or guard it to render only on the first iteration using the loop index.
Ensure the provider button group and its hardcoded IDs are rendered exactly once
while preserving all banner link rendering.
In `@layouts/partials/hooks/body-end.html`:
- Line 14: Add /js/ask-ai-accessibility.js to the body-end script includes in
layouts/partials/hooks/body-end.html alongside the existing ask-ai widget
partial, ensuring it loads after the widget so `#ask-ai-button` receives its
dynamic aria-expanded and aria-controls behavior.
---
Nitpick comments:
In `@layouts/partials/footer.html`:
- Line 110: Change the id prefix in the AI provider button anchor within the
ai-buttons template from footer-btn- to a distinct prefix such as
footer-ai-btn-. Keep the label-based suffix unchanged, and leave the
footer-buttons IDs untouched.
- Around line 99-124: Remove the trailing whitespace emitted by the ariaLabel
conditional in the absolute-link template, while preserving the conditional
aria-label attribute and surrounding link attributes.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 848720cd-142c-4f94-93db-a2499570becc
⛔ Files ignored due to path filters (6)
assets/icons/ai.svgis excluded by!**/*.svgassets/icons/chatgpt.svgis excluded by!**/*.svgassets/icons/claude.svgis excluded by!**/*.svgassets/icons/gemini.svgis excluded by!**/*.svgassets/icons/grok.svgis excluded by!**/*.svgassets/icons/perplexity.svgis excluded by!**/*.svg
📒 Files selected for processing (7)
assets/scss/_styles_project.scssassets/scss/ask-ai-button.scsslayouts/partials/ask-ai-button.htmllayouts/partials/ask-ai-widget.htmllayouts/partials/footer.htmllayouts/partials/hooks/body-end.htmllayouts/partials/navbar.html
| class="gd-docs-header-nav__cta gd-docs-header-nav__secondary open-kapa-widget ask-ai-button" | ||
| aria-expanded="false" | ||
| aria-haspopup="dialog" | ||
| aria-controls="kapa-modal-content" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Remove static aria-controls — the controlled element doesn't exist on initial load.
aria-controls="kapa-modal-content" is set in the HTML, but the kapa-modal-content element lives inside the Kapa widget's shadow DOM and won't exist until the async script loads and the user opens the modal. The accessibility JS in static/js/ask-ai-accessibility.js already adds aria-controls when the modal appears and removes it when it disappears. The static value is redundant and incorrectly references a non-existent element on initial page load, which can confuse screen readers.
♿ Proposed fix
aria-expanded="false"
aria-haspopup="dialog"
- aria-controls="kapa-modal-content"
id="ask-ai-button"📝 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.
| aria-controls="kapa-modal-content" |
🤖 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 `@layouts/partials/ask-ai-button.html` at line 12, Remove the static
aria-controls="kapa-modal-content" attribute from the ask-AI button markup. Keep
the dynamic handling in ask-ai-accessibility.js unchanged so it adds the
attribute when the modal appears and removes it when the modal disappears.
| {{ with $link.subtitle }} | ||
| <p class="c-p mt-lg-4 mb-1 mb-lg-3">{{ . }}</p> | ||
| {{ end }} | ||
| {{ template "ai-buttons" }} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
ai-buttons template is rendered inside the bannerLinks range loop, causing duplicate IDs when multiple banner links exist.
{{ template "ai-buttons" }} at line 17 is inside {{ range $index, $link := $footer.footer.bannerLinks }}. The ai-buttons template (line 110) generates hardcoded IDs like footer-btn-chatgpt, footer-btn-claude, etc. If bannerLinks contains more than one item, these IDs — and the entire provider button group — will be duplicated in the DOM, producing invalid HTML and potential accessibility/JS issues.
Consider moving the ai-buttons call outside the range loop, or guarding it to render only once (e.g., {{ if eq $index 0 }}{{ template "ai-buttons" }}{{ end }}).
🤖 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 `@layouts/partials/footer.html` at line 17, Move the ai-buttons template
invocation outside the bannerLinks range loop, or guard it to render only on the
first iteration using the loop index. Ensure the provider button group and its
hardcoded IDs are rendered exactly once while preserving all banner link
rendering.
| <script src="/js/prism-accessibility.js"></script> | ||
| {{ $themeSwitcherJS := resources.Get "js/theme-switcher.js" | minify | fingerprint }} | ||
| <script src="{{ $themeSwitcherJS.RelPermalink }}" integrity="{{ $themeSwitcherJS.Data.integrity }}" defer></script> | ||
| {{ partial "ask-ai-widget.html" . }} |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for where ask-ai-accessibility.js is loaded in the codebase.
rg -n "ask-ai-accessibility" --type=html --type=mdRepository: gooddata/gooddata-docs-theme
Length of output: 166
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== references to ask-ai =="
rg -n --hidden --glob '!**/.git/**' "ask-ai-accessibility|ask-ai-button|ask-ai-widget" .
echo
echo "== relevant template files =="
git ls-files 'layouts/**' 'static/js/**' | rg 'body-end|ask-ai|hooks|partials|js'
echo
echo "== body-end template =="
sed -n '1,220p' layouts/partials/hooks/body-end.htmlRepository: gooddata/gooddata-docs-theme
Length of output: 3405
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== ask-ai-widget partial =="
sed -n '1,220p' layouts/partials/ask-ai-widget.html
echo
echo "== custom-scripts partial =="
sed -n '1,220p' layouts/partials/custom-scripts.html
echo
echo "== any direct include of ask-ai-accessibility.js =="
rg -n "/js/ask-ai-accessibility\.js|ask-ai-accessibility\.js" layouts static assetsRepository: gooddata/gooddata-docs-theme
Length of output: 2308
Load /js/ask-ai-accessibility.js with the other body-end scripts.
layouts/partials/hooks/body-end.html includes the widget partial, but not the accessibility helper. Without it, #ask-ai-button won’t get the dynamic aria-expanded / aria-controls behavior.
🤖 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 `@layouts/partials/hooks/body-end.html` at line 14, Add
/js/ask-ai-accessibility.js to the body-end script includes in
layouts/partials/hooks/body-end.html alongside the existing ask-ai widget
partial, ensuring it loads after the widget so `#ask-ai-button` receives its
dynamic aria-expanded and aria-controls behavior.
Navbar gains an "Ask AI" button that opens the Kapa AI widget, loaded asynchronously from the body-end hook. Both render only when params.askAI.websiteId is configured; Kapa's own launcher stays hidden and theming (colors, logo, fonts) is overridable via params.askAI.* with GoodData-branded defaults. Cmd+K also opens the modal.
1f52619 to
d125983
Compare
czechian
changed the title
1 participant
Summary
Proof of concept for an AI assistant entry point in the docs theme (footer AI provider links were split out into their own PR):
layouts/partials/ask-ai-button.html) — opens the Kapa AI widget via itsopen-kapa-widgetoverride class.layouts/partials/ask-ai-widget.html, included from the body-end hook) — asyncwidget.kapa.aibundle; Kapa's own launcher stays hidden (data-button-hide), Cmd+K opens the modal, colors/logo/fonts overridable viaparams.askAI.*with GoodData-branded defaults.Everything is config-gated: sites without
params.askAI.websiteIdget no button and no Kapa script.Verification
Built cleanly with Hugo 0.119 (current production version); the Kapa script is correctly absent while
websiteIdis unset in the theme's demo config.Notes
params.askAI.websiteIdafter picking this theme version up.🤖 Generated with Claude Code