Sanity#79
Conversation
✅ Deploy Preview for develop-devlovers ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughThe changes refactor blog components by introducing portable text types for rich-text content, replacing absolute with relative imports, moving inline helper logic into components, enhancing the search UI with autocomplete suggestions and keyboard interactions, and applying refined styling across multiple elements. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
frontend/components/blog/BlogCard.tsx (2)
17-22: Consider extracting excerpt generation to a reusable helper.The inline excerpt extraction logic is correct but spans six lines with nested operations. For better maintainability and testability, consider extracting this to a separate function (e.g.,
generateExcerpt(body: PortableText | undefined): string).🔎 Proposed refactor
Create a helper function in a shared utilities file or at the top of this file:
+function generateExcerpt(body?: PortableText, maxLength = 160): string { + return ( + body + ?.filter((b): b is PortableTextBlock => b?._type === 'block') + .map(b => (b.children || []).map(c => c.text || '').join(' ')) + .join(' ') + .slice(0, maxLength) || '' + ); +} + export default function BlogCard({ post, selectedTags, onTagToggle, }: { post: Post; selectedTags: string[]; onTagToggle: (tag: string) => void; }) { - const excerpt = - post.body - ?.filter((b): b is PortableTextBlock => b?._type === 'block') - .map(b => (b.children || []).map(c => c.text || '').join(' ')) - .join(' ') - .slice(0, 160) || ''; + const excerpt = generateExcerpt(post.body);
59-59: Move font-family to Tailwind configuration.The inline
styleprop breaks the styling convention established by Tailwind classes elsewhere. Define this font stack intailwind.config.jsundertheme.extend.fontFamilyand reference it via a class (e.g.,font-rounded).🔎 Proposed refactor
In
tailwind.config.js:module.exports = { theme: { extend: { fontFamily: { rounded: ['ui-rounded', 'system-ui', '-apple-system', 'sans-serif'], }, }, }, };Then update the component:
className=" text-[22px] font-semibold + font-rounded text-gray-900 leading-snug hover:text-gray-700 transition " - style={{ fontFamily: 'ui-rounded, system-ui, -apple-system' }} >
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
frontend/package-lock.jsonis excluded by!**/package-lock.jsonstudio/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
frontend/components/blog/BlogCard.tsxfrontend/components/blog/BlogFilters.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/components/blog/BlogCard.tsx (1)
frontend/components/blog/BlogFilters.tsx (1)
PortableTextBlock(17-21)
🔇 Additional comments (13)
frontend/components/blog/BlogCard.tsx (5)
5-6: LGTM: Relative imports improve module cohesion.The shift to relative imports for co-located components is appropriate and PortableTextBlock is correctly exported from BlogFilters.
25-36: LGTM: Enhanced card styling with smooth transitions.The custom shadow utilities provide refined visual depth, and the transition on hover improves the user experience.
38-45: LGTM: Responsive aspect ratio improves layout consistency.The change from fixed height (
h-56) toaspect-[16/9]ensures consistent image proportions across different viewport sizes, and the gray background provides a better loading experience.
113-117: LGTM: Conditional rendering prevents empty paragraph elements.Checking for
excerptexistence before rendering improves the DOM structure and avoids unnecessary layout shifts.
124-136: LGTM: Accessible full-width button with clear visual feedback.The button styling provides appropriate visual hierarchy and hover states, improving the overall user experience.
frontend/components/blog/BlogFilters.tsx (8)
37-37: LGTM: Type safety improved by replacinganywithPortableText.Replacing
anyandany[]with the structuredPortableTexttype provides better compile-time safety and IDE support.Also applies to: 50-50
62-76: LGTM: Properly implemented decorative icon component.The
SearchIconcorrectly usesaria-hidden="true"to hide the decorative SVG from assistive technologies, and the flexibleclassNameprop allows for styling customization.
107-113: LGTM: Efficient tag aggregation with proper memoization.Using
useMemowith aSetfor deduplication andnormalizeTagfor consistency is the right approach. The alphabetical sort ensures predictable autocomplete suggestions.
115-127: LGTM: Autocomplete logic correctly implements prefix matching.The suggestion algorithm efficiently finds the first alphabetically-matching tag and correctly computes the remainder for inline display. The memoization prevents unnecessary recalculations on each render.
161-176: LGTM: Keyboard interactions enhance autocomplete UX.The Tab-to-complete and Enter-to-add interactions are correctly implemented with proper guards and
preventDefaultcalls. This provides a smooth typing experience.
193-242: LGTM: Tag controls provide clear, accessible interactions.The conditional rendering logic correctly uses
inputTriminstead of rawinput, and thetitleattribute on the remove button (line 207) aids discoverability. The consistent gray styling aligns well with the overall design.
253-257: LGTM: Clear empty state messaging.The no-results message provides helpful feedback when filters return no matches, improving the user experience.
12-29: Consider making PortableText types more comprehensive to match Sanity schema.The type definitions are incomplete compared to your Sanity blockContent schema. Sanity blocks include
style,marks, andlevelfields, and images include additional properties, but your types only define the minimal subset currently used (_type,text,children,url). While this works for current code that only extracts plain text, it creates maintenance risk. If code expands to access schema fields likestyleormarks, those accesses won't be type-safe. Additionally, PostDetails.tsx and AuthorModal.tsx typebioandbodyasanyinstead of using the exported types from BlogFilters.tsx. Consider aligning your type definitions with Sanity's actual structure or documenting why the minimal subset is intentional.
| {post.tags.map((tag, i) => { | ||
| const norm = normalizeTag(tag); | ||
| const norm = tag.toLowerCase(); | ||
| const active = selectedTags.includes(norm); |
There was a problem hiding this comment.
Inconsistent tag normalization—use the shared normalizeTag helper.
Line 91 uses tag.toLowerCase(), but BlogFilters.tsx exports a more comprehensive normalizeTag function that strips #, trims whitespace, lowercases, and normalizes internal spaces. This inconsistency can cause tag-matching bugs if tags have leading/trailing whitespace or other variations.
🔎 Proposed fix
Import and use the existing helper:
import AuthorModal from './AuthorModal';
-import type { Post, PortableTextBlock } from './BlogFilters';
+import type { Post, PortableTextBlock } from './BlogFilters';
+import { normalizeTag } from './BlogFilters';
export default function BlogCard({
post,
selectedTags,
onTagToggle,
}: {
post: Post;
selectedTags: string[];
onTagToggle: (tag: string) => void;
}) {
const excerpt = // ...
return (
<article ...>
...
{post.tags?.length ? (
<div className="mt-3 flex flex-wrap gap-2">
{post.tags.map((tag, i) => {
- const norm = tag.toLowerCase();
+ const norm = normalizeTag(tag);
const active = selectedTags.includes(norm);📝 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.
| {post.tags.map((tag, i) => { | |
| const norm = normalizeTag(tag); | |
| const norm = tag.toLowerCase(); | |
| const active = selectedTags.includes(norm); | |
| import AuthorModal from './AuthorModal'; | |
| import type { Post, PortableTextBlock } from './BlogFilters'; | |
| import { normalizeTag } from './BlogFilters'; | |
| export default function BlogCard({ | |
| post, | |
| selectedTags, | |
| onTagToggle, | |
| }: { | |
| post: Post; | |
| selectedTags: string[]; | |
| onTagToggle: (tag: string) => void; | |
| }) { | |
| // ... other code ... | |
| {post.tags.map((tag, i) => { | |
| const norm = normalizeTag(tag); | |
| const active = selectedTags.includes(norm); |
🤖 Prompt for AI Agents
In frontend/components/blog/BlogCard.tsx around lines 90 to 92, the code
normalizes tags using tag.toLowerCase(), which is inconsistent with the shared
normalizeTag helper in BlogFilters.tsx; import normalizeTag from its module and
replace tag.toLowerCase() with normalizeTag(tag) so tags are stripped of '#',
trimmed, lowercased and have internal spaces normalized, and ensure selectedTags
uses the same normalization for matching.
| addTag(input); | ||
| } | ||
| }} | ||
| placeholder="Пошук..." |
There was a problem hiding this comment.
Localization inconsistency: placeholder in Ukrainian but UI in English.
The search input placeholder is "Пошук..." (Ukrainian), but all other UI text in this file is hardcoded in English: "Add" (line 224), "Clear" (line 238), "Remove tag" (line 207), and "No posts found for selected tags." (line 255). This creates an inconsistent user experience.
Consider either:
- Using English throughout:
placeholder="Search..." - Implementing proper i18n for all strings using
next-intl(currently imported but unused)
🔎 Quick fix for consistency
- placeholder="Пошук..."
+ placeholder="Search..."📝 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.
| placeholder="Пошук..." | |
| placeholder="Search..." |
🤖 Prompt for AI Agents
In frontend/components/blog/BlogFilters.tsx around line 177 (and nearby lines
207, 224, 238, 255), the placeholder is in Ukrainian while other UI strings are
in English, causing localization inconsistency; fix by making strings
consistent: either replace placeholder="Пошук..." with placeholder="Search..."
to keep English everywhere, or preferably use the existing next-intl import to
internationalize all hardcoded strings (replace literal strings "Add", "Clear",
"Remove tag", "No posts found for selected tags.", and the placeholder with
calls to intl.formatMessage or <FormattedMessage> using appropriate message
keys) and ensure next-intl is actually used and message keys are added to the
locale files.
Summary by CodeRabbit
Release Notes
New Features
Style
✏️ Tip: You can customize this high-level summary in your review settings.