feat: polish documentation layout and add styles for improved UI#40
Open
srimon12 wants to merge 3 commits into
Open
feat: polish documentation layout and add styles for improved UI#40srimon12 wants to merge 3 commits into
srimon12 wants to merge 3 commits into
Conversation
📝 WalkthroughWalkthroughThis PR replaces the Minima Jekyll theme with a custom layout and dark stylesheet, adds front matter to docs to use the new layout, and adds a Healthcare Conversation RAG example README. ChangesDocumentation Site Redesign
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…nd styling adjustments
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/_layouts/default.html (1)
61-95:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard Clipboard API availability and handle execCommand’s success return
Line 64 can throw synchronously whennavigator.clipboardis missing (Clipboard API is secure-context dependent), so the fallback may never run. Line 79 ignoresdocument.execCommand('copy')’s boolean return; use it to avoid showing “Copied!” when the copy command wasn’t successfully invoked (even though the boolean isn’t a perfect guarantee of clipboard contents).Suggested patch
btn.addEventListener('click', () => { const code = pre.querySelector('code'); const text = code ? code.innerText : pre.innerText; - navigator.clipboard.writeText(text).then(() => { + const markCopied = () => { btn.textContent = 'Copied!'; btn.classList.add('copied'); setTimeout(() => { btn.textContent = 'Copy'; btn.classList.remove('copied'); }, 2000); - }).catch(() => { + }; + + const markFailed = () => { btn.textContent = 'Copy failed'; btn.classList.add('copy-error'); setTimeout(() => { btn.textContent = 'Copy'; btn.classList.remove('copy-error'); }, 2000); }; + + const fallbackCopy = () => { const ta = document.createElement('textarea'); ta.value = text; ta.style.position = 'fixed'; ta.style.opacity = '0'; document.body.appendChild(ta); ta.select(); try { - document.execCommand('copy'); - btn.textContent = 'Copied!'; - btn.classList.add('copied'); - setTimeout(() => { - btn.textContent = 'Copy'; - btn.classList.remove('copied'); - }, 2000); + const copied = document.execCommand('copy'); + if (copied) markCopied(); + else markFailed(); } catch (e) { - btn.textContent = 'Copy failed'; - btn.classList.add('copy-error'); - setTimeout(() => { - btn.textContent = 'Copy'; - btn.classList.remove('copy-error'); - }, 2000); + markFailed(); + } finally { + document.body.removeChild(ta); } - document.body.removeChild(ta); - }); + }; + + if (navigator.clipboard?.writeText) { + navigator.clipboard.writeText(text).then(markCopied).catch(fallbackCopy); + } else { + fallbackCopy(); + } });🤖 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 `@docs/_layouts/default.html` around lines 61 - 95, Guard against missing Clipboard API before calling navigator.clipboard.writeText by checking for navigator.clipboard and navigator.clipboard.writeText and wrap the async path in try/catch so a synchronous exception can't bypass the fallback; in the fallback path use the boolean return of document.execCommand('copy') to determine success (only set "Copied!" and add 'copied' when execCommand returns true) and treat false as a failure (set "Copy failed" and add 'copy-error'); ensure the textarea (ta) is removed in all code paths. Reference: the click handler closure that uses btn, navigator.clipboard.writeText, and document.execCommand('copy').
🤖 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.
Outside diff comments:
In `@docs/_layouts/default.html`:
- Around line 61-95: Guard against missing Clipboard API before calling
navigator.clipboard.writeText by checking for navigator.clipboard and
navigator.clipboard.writeText and wrap the async path in try/catch so a
synchronous exception can't bypass the fallback; in the fallback path use the
boolean return of document.execCommand('copy') to determine success (only set
"Copied!" and add 'copied' when execCommand returns true) and treat false as a
failure (set "Copy failed" and add 'copy-error'); ensure the textarea (ta) is
removed in all code paths. Reference: the click handler closure that uses btn,
navigator.clipboard.writeText, and document.execCommand('copy').
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 594e2d56-bf1c-4778-a533-55540f2b2470
📒 Files selected for processing (6)
docs/_layouts/default.htmldocs/assets/css/style.cssdocs/collections.mddocs/reference.mddocs/search.mdexamples_and_applications/healthcare_conversation_rag/README.md
✅ Files skipped from review due to trivial changes (4)
- docs/reference.md
- docs/collections.md
- examples_and_applications/healthcare_conversation_rag/README.md
- docs/search.md
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/_layouts/default.html (1)
57-58: ⚡ Quick winConsider adding an explicit aria-label for improved accessibility.
The copy button relies on its text content ("Copy") as the label, which works but could be more descriptive for screen reader users. Adding an explicit
aria-labelwould clarify the button's purpose.♿ Proposed enhancement for accessibility
const btn = document.createElement('button'); btn.className = 'copy-btn'; btn.textContent = 'Copy'; +btn.setAttribute('aria-label', 'Copy code to clipboard');🤖 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 `@docs/_layouts/default.html` around lines 57 - 58, The copy button created as btn (class 'copy-btn') lacks an explicit aria-label; update the element creation for btn to set a descriptive aria-label (e.g., "Copy code to clipboard" or similar) using btn.setAttribute('aria-label', '...') or btn.ariaLabel so screen readers get a clear purpose for the button while preserving existing text content and behavior.
🤖 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.
Nitpick comments:
In `@docs/_layouts/default.html`:
- Around line 57-58: The copy button created as btn (class 'copy-btn') lacks an
explicit aria-label; update the element creation for btn to set a descriptive
aria-label (e.g., "Copy code to clipboard" or similar) using
btn.setAttribute('aria-label', '...') or btn.ariaLabel so screen readers get a
clear purpose for the button while preserving existing text content and
behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e230394b-5ab0-42f0-bd4d-f24340f9f4af
📒 Files selected for processing (1)
docs/_layouts/default.html
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
1 participant
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.
Summary
Replaced the default Minima Jekyll theme with a custom dark-themed documentation layout for a polished, professional look.
Changes
docs/_layouts/default.html) — dark sidebar with navigation, GitHub/PyPI links, and active page highlightingdocs/assets/css/style.css) — GitHub-dark inspired palette (#0d1117 background, #161b22 sidebar), responsive typography, styled tables, blockquotes, and code blockstheme: minimafrom_config.ymlTesting
Summary by CodeRabbit
Documentation
New Features