fix: link not getting highlighted #4509
Conversation
WalkthroughIntroduces website/static/js/blog.js to handle blog search filtering, empty-state rendering, link styling within blog content, smooth scroll to comments, and card hover effects. Updates templates to load the new script. post_detail adds a blog-content class. post_list removes inline JS and updates “Read more” link styling. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant DOM as Document
participant JS as blog.js
participant Grid as Blog Grid
participant Empty as Empty State
User->>DOM: Load page
DOM-->>JS: DOMContentLoaded
JS->>DOM: Bind input on #post-search
JS->>DOM: Enhance links in .blog-content
JS->>DOM: Bind hover on .blog-card
JS->>DOM: Bind click on [href="#comments"]
User->>DOM: Type in #post-search
DOM-->>JS: input event
JS->>Grid: Show/Hide .blog-card based on match
JS->>JS: checkEmptyState()
alt No visible cards and non-empty query
JS->>Empty: Create/Attach empty-state node
else Results visible or empty query
JS->>Empty: Remove empty-state node if present
end
User->>DOM: Click #comments link
JS->>DOM: scrollIntoView(#comment_root, smooth)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
website/static/js/blog.js (6)
18-24: Avoid forcing display:block; toggle only 'hidden' to reduce layout side-effectsCards already have their display semantics. Forcing
blockcan conflict with grid/flex layouts. Toggle the visibility class only.- if (title.includes(searchTerm) || author.includes(searchTerm)) { - card.classList.remove('hidden'); - card.classList.add('block'); - } else { - card.classList.remove('block'); - card.classList.add('hidden'); - } + const matches = title.includes(searchTerm) || author.includes(searchTerm); + card.classList.toggle('hidden', !matches);
10-28: Optional: debounce input to avoid excessive DOM churn on fast typingDebouncing improves input responsiveness on long lists. Keep delay small (100–150ms).
- searchInput.addEventListener('input', function() { - const searchTerm = this.value.toLowerCase().trim(); + const debounce = (fn, delay = 120) => { + let t; + return (...args) => { clearTimeout(t); t = setTimeout(() => fn.apply(null, args), delay); }; + }; + searchInput.addEventListener('input', debounce(() => { + const searchTerm = searchInput.value.toLowerCase().trim(); ... - checkEmptyState(); - }); + checkEmptyState(); + }));
31-50: Guard against missing grid on pages without a grid; align empty-state color with site brand
- If
.blog-gridisn’t present,appendChildwill throw.- The rest of the UI uses brand red
#e74c3c; consider aligning the empty state icon color.function checkEmptyState() { const visibleCards = document.querySelectorAll('.blog-card:not(.hidden)'); const grid = document.querySelector('.blog-grid'); const existingEmptyState = document.querySelector('.empty-state-message'); - if (visibleCards.length === 0 && !existingEmptyState && searchInput.value.trim() !== '') { + if (!grid) { return; } + if (visibleCards.length === 0 && !existingEmptyState && searchInput.value.trim() !== '') { const emptyState = document.createElement('div'); emptyState.className = 'empty-state-message col-span-full flex flex-col items-center justify-center py-16 text-center'; emptyState.innerHTML = ` - <svg class="w-16 h-16 text-red-500 opacity-50 mb-4" xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" stroke="currentColor"> + <svg class="w-16 h-16 text-[#e74c3c] opacity-50 mb-4" xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" stroke="currentColor"> <path stroke-linecap="round" stroke-linejoin="round" stroke-width="1" d="M21 21l-6-6m2-5a7 7 0 11-14 0 7 7 0 0114 0z" /> </svg> <h2 class="text-xl font-semibold text-gray-700 mb-2">No matching posts found</h2> <p class="text-gray-500 max-w-md">Try adjusting your search criteria or browse all posts below.</p> `; grid.appendChild(emptyState); } else if ((visibleCards.length > 0 || searchInput.value.trim() === '') && existingEmptyState) { existingEmptyState.remove(); } }
52-72: Prefer CSS classes over inline styles and JS hover for content links; add base underline and visited statesInline styles and JS hover handlers fight theming, visited color, and accessibility. Tailwind classes (or a small CSS rule) are sufficient.
const links = blogContent.querySelectorAll('a'); links.forEach(link => { // Apply blue styling to all links in blog content - link.classList.add('text-blue-600', 'hover:text-blue-800', 'hover:underline'); - link.style.color = '#2563eb'; // Ensures the color is applied even if CSS classes don't load - - // Add hover event listeners for better control - link.addEventListener('mouseenter', function() { - this.style.color = '#1d4ed8'; - this.style.textDecoration = 'underline'; - }); - - link.addEventListener('mouseleave', function() { - this.style.color = '#2563eb'; - this.style.textDecoration = 'none'; - }); + link.classList.add( + 'text-blue-600', + 'visited:text-purple-600', + 'hover:text-blue-800', + 'underline', + 'underline-offset-2', + 'focus:outline-none', + 'focus-visible:ring-2', + 'focus-visible:ring-blue-300', + 'rounded-sm' + ); });Note: if Tailwind’s
visited:orfocus-visible:variants aren’t enabled, add a small CSS rule instead:.blog-content a { color:#2563eb; text-decoration:underline; } .blog-content a:hover { color:#1d4ed8; } .blog-content a:visited { color:#7c3aed; }
75-87: Make “scroll to comments” resilient; prefer the href target and only fall back to comment_rootCurrent code prevents default and targets
comment_rootunconditionally. Use the link’s hash if present, else fall back.- const commentLinks = document.querySelectorAll('[href="#comments"]'); + const commentLinks = document.querySelectorAll('a[href="#comments"]'); commentLinks.forEach(link => { link.addEventListener('click', function(e) { - e.preventDefault(); - const commentsSection = document.getElementById('comment_root'); - if (commentsSection) { - commentsSection.scrollIntoView({ + const targetSelector = this.getAttribute('href'); // "#comments" + const targetEl = document.querySelector(targetSelector) || document.getElementById('comment_root'); + if (targetEl) { + e.preventDefault(); + targetEl.scrollIntoView({ behavior: 'smooth', block: 'start' }); } }); });
90-99: Use CSS hover for card scale; drop JS listenersThis effect is declarative in CSS and avoids many listeners.
- // Blog card hover effects - const cards = document.querySelectorAll('.blog-card'); - cards.forEach(card => { - card.addEventListener('mouseenter', function() { - this.classList.add('transform', 'scale-105'); - }); - - card.addEventListener('mouseleave', function() { - this.classList.remove('transform', 'scale-105'); - }); - }); + // Tip: Prefer CSS utility classes on the card element: hover:scale-105 + // (See template change suggestion in post_list.html review.)Additional template change (outside this file) to apply:
<div class="blog-card ... transform transition duration-300 hover:-translate-y-1 hover:scale-105 hover:shadow-xl">website/templates/blog/post_list.html (1)
168-169: Load blog.js here (OK); add a semantic author hook to stabilize searchIncluding the external script is good. To make author filtering robust and decoupled from classes, add a data attribute on the author span; update blog.js to prefer
[data-author].Outside this hunk, adjust the author badge span:
<span class="text-white text-sm" data-author="{{ post.author }}">{{ post.author }}</span>Then the selector in blog.js (already suggested) continues to work even if classes change.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge Base: Disabled due to Reviews > Disable Knowledge Base setting
📒 Files selected for processing (3)
website/static/js/blog.js(1 hunks)website/templates/blog/post_detail.html(2 hunks)website/templates/blog/post_list.html(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run Tests
- GitHub Check: docker-test
🔇 Additional comments (5)
website/static/js/blog.js (2)
4-4: Good initialization guard with DOMContentLoadedEncapsulating all bindings under DOMContentLoaded avoids timing issues across templates. LGTM.
39-45: Empty-state copy is clear; good UX touchConcise and helpful guidance when no matches are found. Nice.
website/templates/blog/post_detail.html (2)
25-25: Adding blog-content hook is the right approachThis enables targeted styling of in-article links without affecting the rest of the page. LGTM.
50-50: blog.js inclusion verified; comment_root present
- blog.js is only included once in
website/templates/blog/post_detail.html, so there’s no risk of double-bound listeners.- An element with
id="comment_root"is confirmed inwebsite/templates/comments2.html, ensuring the script’s anchor exists.Optional nit: you may add
deferto the script tag to prevent parser blocking, e.g.:- <script src="{% static 'js/blog.js' %}"></script> + <script defer src="{% static 'js/blog.js' %}"></script>website/templates/blog/post_list.html (1)
120-120: Link styling matches the “highlight link” objectiveSwitching “Read more” to blue aligns with the new in-article link color, improving consistency. LGTM.
fixes #4508
Summary by CodeRabbit