Conversation
This commit introduces several new font packages to the project: - `@fontsource-variable/inter` - `@fontsource-variable/jetbrains-mono` - `@fontsource-variable/source-serif-4` Additionally, it includes a `SafeHtmlRenderer` component to sanitize and render HTML content safely, and integrates this into the article view to prevent potential XSS vulnerabilities. The `min-w-[200px]` class in `editor-bubble-menu.tsx` has been shortened to `min-w-50` for a more concise style. The `devtools` plugin in `vite.config.ts` is now conditionally enabled only in development mode to prevent hydration mismatches.
Introduces `ArticleMoreButton` and `ArticleShareButton` for article interactions. Refactors `ContentBar` to use these new components and the `useToggleArticleLike` hook. Updates `ArticleView` to pass necessary props to `ContentBar`. Adds article query options and related utilities for data fetching and management. Implements route context for `QueryClient` and user data. Sets up route-based data fetching and error handling for the article page.
This commit introduces significant changes to the article writing and editing experience. Key changes include: - **Renamed "Story" to "Article"**: The terminology has been updated for consistency across the application. - **New Edit Component**: A new `EditComponent` has been created to manage the article editing interface. - **Edit Navbar and Update Button**: Introduced `EditNavbar` and `UpdateButton` components to facilitate saving and updating article drafts. - **Refactored Publish Button**: The `PublishButton` has been refactored to use a dedicated `usePublishArticle` hook for better data management. - **New Update Article Hook**: Added `useUpdateArticle` hook for handling article updates. - **New Edit Route**: Created a new route `/article/edit/$slug` for editing existing articles, including author and authentication checks. - **Placeholder Text Update**: The placeholder text in the editor has been changed from "Tell your story..." to "Write your article...". - **Image Upload Prompt Update**: The prompt for image uploads has been updated to refer to "article" instead of "story".
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughThis PR introduces optional authentication for article viewing, HTML content sanitization, article liking functionality with user-aware state tracking, new article management components (view/edit/share), route restructuring to separate article view and edit paths, updated styling with new font sources, and dependency additions. Most backend files are normalized to use single quotes for consistency. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Client as Browser Client
participant Router as TanStack Router
participant QueryClient as React Query
participant API as Backend API
participant DB as Database
User->>Client: Navigate to article slug
Client->>Router: Route to /_main/article/$slug
Router->>QueryClient: loader calls GetArticleQueryOptions(slug)
QueryClient->>API: GET /api/v1/articles/{slug}
API->>DB: Query article by slug
DB-->>API: Return article data
API-->>QueryClient: Return article + isLikedByUser
QueryClient-->>Router: Cache article data
Router-->>Client: Data loaded
Client->>Client: RouteComponent renders
Client->>Client: useSuspenseQuery(GetArticleQueryOptions(slug))
Client->>Client: Obtain user from route context
Client->>Client: Render ArticleView(article, user)
Client-->>User: Display article with like state
User->>Client: Click like button
Client->>QueryClient: useToggleArticleLike mutation
QueryClient->>API: POST /api/v1/likes
API->>DB: Insert/delete like record
DB-->>API: Success
API-->>QueryClient: Return response message
QueryClient->>QueryClient: Invalidate article queries
QueryClient-->>Client: Refetch article data
Client-->>User: Update like count + heart icon
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
backend/src/services/article.service.ts (1)
178-185:⚠️ Potential issue | 🟠 MajorContent is not sanitized on update, unlike on create.
CreatePostsanitizescontentviasanitizeHtml(content)(Line 37), butUpdatePostspreadsbodydirectly into.set()without sanitizing. SinceUpdatePostBody = CreatePostBody.partial(), thecontentfield can be present in an update and will be stored unsanitized, bypassing the protection added inCreatePost.🔒 Proposed fix
const [updatedPost] = await db .update(article) .set({ - ...body, + ...body, + ...(body.content ? { content: sanitizeHtml(body.content) } : {}), updatedAt: new Date(), }) .where(eq(article.id, id)) .returning()You'll also need to add
sanitizeHtmlto the imports used byUpdatePost(it's already imported at Line 10).backend/src/controllers/article.controller.ts (1)
32-46:⚠️ Potential issue | 🟡 MinorPUT endpoint already updated to new path; DELETE feature not yet implemented in frontend.
The PUT route change to
/article/id/:idis confirmed handled—the frontend mutation atui/src/data/queries/article.ts(lines 66–68) correctly uses.id({ id }).put(data). However, the DELETE endpoint exists in the backend but lacks a corresponding mutation hook in the frontend. ThehandleDeletefunction inui/src/components/article/article-more-button.tsx(line 46) is currently unimplemented (only logs to console). Implement the delete mutation before the DELETE route becomes a breaking change for actual API consumers.
🤖 Fix all issues with AI agents
In `@backend/src/lib/cloudinary.ts`:
- Line 16: The Cloudinary config currently sets secure: false which forces HTTP;
update the configuration in backend/src/lib/cloudinary.ts (the Cloudinary config
object / cloudinary.v2.config call) to set secure: true so uploads/URLs use
HTTPS, and verify any related environment-driven value (e.g., CLOUDINARY_SECURE)
or configuration loader reflects true in production; run a quick test to confirm
generated URLs are https.
In `@backend/src/lib/sanitize-html.ts`:
- Line 35: The ALLOWED_URI_REGEXP constant in sanitize-html.ts is overly
permissive and can bypass DOMPurify's default URI protections; remove the custom
ALLOWED_URI_REGEXP entry from the sanitizer config so DOMPurify's built-in URI
filtering is used, or if you explicitly need only certain schemes replace the
value with a strict allowlist regex that only permits
^(?:https?|mailto|tel|ftp): and nothing else (update any references to
ALLOWED_URI_REGEXP accordingly, e.g., where sanitizeHtml or sanitizerConfig is
constructed).
- Line 30: The project depends on a pre-release of isomorphic-dompurify
(currently ^3.0.0-rc.2); update the dependency to a stable, pinned version
(e.g., "isomorphic-dompurify": "2.35.0") in package.json and regenerate the
lockfile so sanitize functions in backend/src/lib/sanitize-html.ts import a
stable release; if v3 is required, pin the exact v3 RC tag instead of using a
caret range to avoid accidental upgrades.
In `@package.json`:
- Around line 15-18: Add "elysia" as a direct dependency in the backend
package.json (so backend code that imports elysia in files like
backend/src/server.ts and controllers/middleware/services resolves correctly)
and remove the unnecessary "marklink-monorepo": "." entry from the root
package.json; update backend/package.json's "dependencies" to include the
appropriate elysia semver and delete the root-level "marklink-monorepo"
dependency entry to avoid the implicit hoisting reliance.
In `@ui/src/components/article/article-share-button.tsx`:
- Around line 27-30: The shareUrl is being built from articleID which yields
broken links because routing uses slug; update the shareUrl construction in
article-share-button (the shareUrl constant) to use the article's slug instead
of articleID (or accept a slug prop if the component currently only receives
articleID). Locate where articleID is passed into the component and replace
usage in shareUrl with slug (or add a new slug prop and use
`${window.location.origin}/article/${slug}` with the same window check) so
shared links point to the routed slug path.
In `@ui/src/components/article/safe-html-renderer.tsx`:
- Around line 35-44: The SafeHtmlRenderer currently injects htmlContent directly
via dangerouslySetInnerHTML; add a client-side DOMPurify sanitization step as a
last line of defense: import DOMPurify, compute a memoized safeHtml =
DOMPurify.sanitize(htmlContent) (e.g., via useMemo) inside the SafeHtmlRenderer
component, and replace dangerouslySetInnerHTML={{ __html: htmlContent }} with
dangerouslySetInnerHTML={{ __html: safeHtml }}; ensure you keep the existing
containerRef and className usage and add a short JSDoc on the htmlContent prop
stating it should be sanitized upstream as well.
In `@ui/src/components/Editor/use-editor.tsx`:
- Line 17: The mapping for the editor option is inverted: currently you pass
immediatelyRender: ssr which sets immediatelyRender true during SSR and causes
hydration mismatches; update the code that builds the Tiptap options in
useEditor (the place where ssr?: boolean is declared and passed) to set
immediatelyRender: !ssr so that SSR (ssr === true) results in immediatelyRender
false; locate the useEditor initialization where immediatelyRender is assigned
and flip the boolean expression from ssr to !ssr.
In `@ui/src/data/queries/query-keys.ts`:
- Around line 1-3: QUERY_KEYS.GET_ARTICLE is currently a static array which
causes nested arrays when used as [QUERY_KEYS.GET_ARTICLE, slug] and breaks
TanStack Query caching; change QUERY_KEYS.GET_ARTICLE into a function factory
that returns a flat tuple (e.g., GET_ARTICLE: (slug) => ['article', slug] as
const) and update the consumer in article query to pass the slug by calling
QUERY_KEYS.GET_ARTICLE(slug) for the queryKey while keeping existing
invalidation calls unchanged.
In `@ui/src/lib/api.ts`:
- Line 5: The client creation uses a hardcoded base URL; update the call to
treaty in client (the exported constant client created via treaty<App>) to read
the base URL from environment/config instead of "http://localhost:3000" (e.g.,
use your build-time env like import.meta.env.VITE_API_URL or
process.env.REACT_APP_API_URL depending on the app) and provide a sensible
fallback (or throw a clear error) so staging/production deploys pick up the
correct endpoint; ensure you update any type or usage that expects the string so
treaty<App>(baseUrl, {...}) still receives a string from the env variable.
In `@ui/src/styles.css`:
- Around line 170-175: The .tiptap pre/code rules are duplicated and
conflicting: remove the pre { ... } (and nested code { ... }) block from inside
the `@layer` base definition so there is a single .tiptap pre/code rule (the
external syntax-highlighting block) that acts as the source of truth; then, in
the external .tiptap pre and .tiptap pre code selectors (the syntax-highlighting
block), add any missing utilities from the removed base version (bg-muted or
bg-muted/50 as desired, text-[15px] or text-[14px], and code text-[14px], plus
any border/font utilities) so all intended properties are consolidated in that
one rule.
- Around line 283-300: Add dark-mode overrides for all remaining highlight token
selectors so their lightness is increased to ~0.75 against the dark pre
background; specifically update the .dark .tiptap pre block to include selectors
such as .hljs-attr, .hljs-built_in, .hljs-type, .hljs-params, .hljs-bullet,
.hljs-symbol, .hljs-link, .hljs-meta, .hljs-number, .hljs-literal, .hljs-doctag,
.hljs-regexp, .hljs-addition, and .hljs-selector-tag with oklch colors matching
the existing pattern (e.g., oklch(0.75 0.15 <hue>)) so all token groups use
higher lightness (~0.75) and maintain consistent hues with the already defined
.hljs-keyword/.hljs-string/.hljs-title/.hljs-variable entries.
🟡 Minor comments (17)
backend/src/services/comment.service.ts-46-48 (1)
46-48:⚠️ Potential issue | 🟡 Minor
InternalServerErrorat Line 28 gets double-wrapped in the catch block.The
InternalServerErrorthrown on Line 28 (whennewCommentis null) will be caught here and re-wrapped in anotherInternalServerError. UnlikeUpdateCommentandDeleteCommentwhich re-throw known error types, this catch block doesn't guard against already-handled errors.Suggested fix
catch (error) { + if (error instanceof InternalServerError) { + throw error + } throw new InternalServerError('Failed to create comment', error) }ui/package.json-17-20 (1)
17-20:⚠️ Potential issue | 🟡 MinorNew font and utility dependencies look reasonable.
The font packages and
dayjsare well-established libraries appropriate for the design system and date formatting needs.However, the dayjs utility file has a typo in its filename:
ui/src/lib/dyajs.tsshould be renamed toui/src/lib/dayjs.tsto match the package name and improve clarity.ui/src/lib/dyajs.ts-1-21 (1)
1-21:⚠️ Potential issue | 🟡 MinorFilename typo:
dyajs.tsshould bedayjs.ts.The file is named
dyajs.ts(transposed letters) instead ofdayjs.ts. This will cause confusion for anyone searching for or importing this module.ui/src/lib/dyajs.ts-10-13 (1)
10-13:⚠️ Potential issue | 🟡 MinorFuture timestamps will pass the
< 7check sincediffreturns a negative value.If
timestampis in the future,diffInDayswill be negative (e.g., -2), which satisfies< 7, causingfromNow()to return strings like "in 2 days". If this is unintended, useMath.abs(diffInDays)or add a guard.backend/package.json-22-23 (1)
22-23:⚠️ Potential issue | 🟡 MinorESLint packages belong in
devDependencies, notdependencies.
@typescript-eslint/eslint-pluginand@typescript-eslint/parserare development-time linting tools. Placing them independenciesmeans they'll be installed in production builds, increasing image/bundle size for no benefit.Proposed fix
"dependencies": { "@bogeychan/elysia-logger": "^0.1.10", "@elysiajs/cors": "^1.4.1", "@elysiajs/openapi": "^1.4.14", "@elysiajs/server-timing": "^1.4.0", - "@typescript-eslint/eslint-plugin": "^8.54.0", - "@typescript-eslint/parser": "^8.54.0", ... }, "devDependencies": { "@antfu/eslint-config": "^7.2.0", + "@typescript-eslint/eslint-plugin": "^8.54.0", + "@typescript-eslint/parser": "^8.54.0", ... }ui/src/components/write/publish-button.tsx-111-112 (1)
111-112:⚠️ Potential issue | 🟡 MinorNote: The
usePublishArticlehook's error toast still says "Failed to publish story".In
ui/src/data/queries/article.ts(Line 55), theonErrorhandler usestoast.error('Failed to publish story')which is inconsistent with the "story → article" terminology update in this file.ui/src/components/article/article-share-button.tsx-7-7 (1)
7-7:⚠️ Potential issue | 🟡 Minor
XingIconis the icon for Xing (a social network), not X (formerly Twitter).Xing is a German professional networking site — this icon won't represent X/Twitter correctly. You likely need a different icon, or an inline SVG for the X logo similar to what was done for Facebook on Line 80.
Also applies to: 85-88
ui/src/lib/types.ts-19-20 (1)
19-20:⚠️ Potential issue | 🟡 MinorChange
createdAtandupdatedAttypes tostring.The API response deserializes dates as ISO string values, not
Dateobjects. The@elysiajs/edenclient doesn't include response transformers, and the query function doesn't convert these fields. WhileformatSmartTime()safely handles strings throughdayjs(), the type definition should accurately reflect the runtime type to prevent misuse elsewhere.ui/src/components/article/content-bar.tsx-41-47 (1)
41-47:⚠️ Potential issue | 🟡 MinorHardcoded
text-gray-900 fill-gray-600will break in dark mode.The rest of the UI uses semantic tokens (
text-foreground,text-muted-foreground). Gray-900 is near-black and will be invisible on dark backgrounds. Use theme-aware tokens instead.💡 Suggested fix
<HugeiconsIcon className={cn( 'size-5', - isLiked && 'text-gray-900 fill-gray-600', + isLiked && 'text-foreground fill-foreground', )} icon={FavouriteIcon} />Adjust the fill token to match your design intent (e.g.,
fill-muted-foregroundfor a subtler look).ui/src/data/queries/article.ts-36-37 (1)
36-37:⚠️ Potential issue | 🟡 Minor
toast.success(data.data?.message)may display an empty toast.If
data.dataisundefined,messagewill also beundefined, resulting in a toast with no visible text. Consider providing a fallback.💡 Proposed fix
- toast.success(data.data?.message) + toast.success(data.data?.message ?? 'Like updated')ui/src/components/write/update-button.tsx-148-179 (1)
148-179:⚠️ Potential issue | 🟡 MinorLabels are not associated with their form controls (accessibility).
The
<label>elements lackhtmlForattributes and the<Textarea>elements lack correspondingidattributes. Screen readers won't associate the labels with their inputs.♿ Proposed fix
- <label - className="text-sm font-medium text-muted-foreground mb-2 - block" - > + <label + htmlFor="update-preview-title" + className="text-sm font-medium text-muted-foreground mb-2 + block" + > Title </label> <Textarea + id="update-preview-title" value={previewTitle}Apply similarly for the "Preview Text" label/textarea pair.
ui/src/data/queries/article.ts-1-1 (1)
1-1:⚠️ Potential issue | 🟡 MinorRemove redundant client-side sanitization in
GetArticleQueryOptions.The backend already sanitizes article content on write (in
CreatePostat backend/src/services/article.service.ts:37), storing sanitized content in the database. Re-sanitizing the same content on every read in the frontend (line 24) is unnecessary and adds no additional security benefit. Remove thesanitizeHtml(res.data.content)call and return the content as-is since it's already sanitized by the backend.Relevant code
queryFn: async () => { const res = await client.api.v1.article({ slug }).get() if (!res.data) throw new Error('Not found') return { ...res.data, content: res.data.content, // Already sanitized by backend } }backend/src/db/schema/auth.ts-37-39 (1)
37-39:⚠️ Potential issue | 🟡 MinorAdd
.defaultNow()tosession.updatedAtandaccount.updatedAtfor consistency and robustness.
session.updatedAt(line 37) andaccount.updatedAt(line 66) lack.defaultNow(), whileuser.updatedAt(line 24) andverification.updatedAt(line 81) include it. All four have.$onUpdate(...).notNull(), but only user and verification provide database-level defaults. Currently, better-auth's drizzleAdapter manages session and account inserts and supplies the value, so there's no immediate runtime issue. However, this inconsistency is fragile: if the schema is later used outside better-auth's control or the integration changes, inserts will fail without explicitupdatedAtvalues. Add.defaultNow()to session and account to match the other tables and ensure the schema is self-contained and defensive.ui/src/routes/_main/article/$slug.tsx-8-13 (1)
8-13:⚠️ Potential issue | 🟡 MinorCatch-all swallows non-404 errors as
notFound().Network failures, 500s, or auth errors from
ensureQueryDatawill all be converted into a "Not found" page. Consider catching only the expected "Not found" error (e.g., checkerror.messageor status code) and re-throwing unexpected errors so they surface properly.ui/src/routes/__root.tsx-41-41 (1)
41-41:⚠️ Potential issue | 🟡 MinorTypo in page title: "Wrire" → "Write".
Proposed fix
- title: 'MarkLink - Read Wrire Share', + title: 'MarkLink - Read Write Share',ui/src/components/article/article-view.tsx-48-52 (1)
48-52:⚠️ Potential issue | 🟡 Minor
bg-coverhas no effect on<img>; useobject-cover.
bg-covercontrolsbackground-size, which doesn't apply to<img>elements. You likely wantobject-coverto maintain aspect ratio while filling the container.Proposed fix
- className="aspect-video bg-cover h-78 w-full" + className="aspect-video object-cover h-78 w-full"ui/src/components/article/article-view.tsx-32-36 (1)
32-36:⚠️ Potential issue | 🟡 MinorEmpty string
srccauses a spurious HTTP request.When
article.author.imageis falsy,src=""makes the browser request the current page URL as an image. Use a placeholder image URL or conditionally render the<img>.Proposed fix
- <img - src={article.author.image || ''} - alt={article.author.name} - className="w-10 h-10 rounded-full object-cover" - /> + {article.author.image ? ( + <img + src={article.author.image} + alt={article.author.name} + className="w-10 h-10 rounded-full object-cover" + /> + ) : ( + <div className="w-10 h-10 rounded-full bg-muted flex items-center justify-center text-sm font-medium"> + {article.author.name?.charAt(0)} + </div> + )}
🧹 Nitpick comments (22)
backend/src/lib/cloudinary.ts (1)
7-11: Consider using the URL API for more robust parsing.While the quote style changes are fine, the manual string parsing with
split('@')andsplit(':')is fragile and lacks validation. The built-inURLAPI would handle edge cases more reliably.♻️ Proposed refactor using URL API
- if (url.startsWith('cloudinary://')) { - const withoutProtocol = url.replace('cloudinary://', '') - const [auth, cloudName] = withoutProtocol.split('@') - if (auth && cloudName) { - const [apiKey, apiSecret] = auth.split(':') + if (url.startsWith('cloudinary://')) { + try { + const parsed = new URL(url) + const cloudName = parsed.hostname + const apiKey = parsed.username + const apiSecret = parsed.password + if (apiKey && apiSecret && cloudName) { cloudinary.config({ cloud_name: cloudName, api_key: apiKey, api_secret: apiSecret, secure: false, upload_preset: 'ml_default', }) + } + } catch (error) { + console.error('Failed to parse CLOUDINARY_URL:', error) } }ui/src/styles.css (1)
116-190: Blanket!importanton nearly every utility inside.tiptap.Every
@applyline uses the!modifier. While this is common for overriding TipTap/ProseMirror defaults, applying!importantto everything (margins, padding, font-size, etc.) makes future style adjustments and component-level overrides difficult. Consider whether a more targeted approach — e.g., increasing selector specificity with.tiptap.ProseMirroror using@layerordering — could replace the blanket!importantfor at least some properties.Not blocking, just something to be mindful of as the editor styles grow.
ui/src/components/layout/main-header.tsx (1)
2-3: Nit:AvaterBtnappears to be a typo forAvatarBtn.The component name and file
avater-btnshould likely beavatar-btn. This pre-dates this PR but would be good to fix while renaming files.ui/src/components/Editor/Editor.tsx (1)
1-23: Import path normalization looks fine, but note partial naming inconsistency.The sibling files have been renamed to lowercase-hyphenated (
editor-bubble-menu,editor-block-menu), but this file and its parent directory retain PascalCase (Editor/Editor.tsx). Consider renaming the directory and file to match for full consistency (e.g.,editor/editor.tsx).ui/src/components/article/comment-view.tsx (1)
1-9: Stub component renders rawarticle_idto the DOM.This appears to be a placeholder. Rendering the raw ID is fine for development, but ensure it's not shipped to production as-is — users shouldn't see internal IDs.
backend/src/middlewares/auth.middleware.ts (1)
16-40: Consider handlinggetSessionfailures distinctly from "no session".If
auth.api.getSession()throws (e.g., session store unavailable), the exception will propagate as a 500 for both macros. InisAuthOptional, this means a backend failure looks like an auth failure to upstream handlers that expectuser: nullfor unauthenticated users. Atry/catchreturning{ user: null }on infrastructure errors would silently swallow outages, so at minimum ensure there's observability (logging) around session lookup failures.backend/src/services/error.service.ts (1)
77-77: Rename misspelled function:SetupOnErorr→SetupOnError.The function name has a transposed letter ("Erorr" instead of "Error"). This requires updating the definition, import, and usage across two files:
backend/src/services/error.service.ts:77(definition)backend/src/controllers/controller.ts:1(import)backend/src/controllers/controller.ts:13(usage)Proposed changes
backend/src/services/error.service.ts:
-export const SetupOnErorr = (error: unknown, set: Context['set']) => { +export const SetupOnError = (error: unknown, set: Context['set']) => {backend/src/controllers/controller.ts:
-import { HttpError, SetupOnErorr } from '@backend/services/error.service.ts' +import { HttpError, SetupOnError } from '@backend/services/error.service.ts'- return SetupOnErorr(error, set) + return SetupOnError(error, set)backend/src/db/schema/article.ts (1)
39-39: Misleading DB column name:liker_idmaps to'author_id'in the database.The JS field is
liker_idbut the underlying Postgres column isauthor_id. This creates a confusing mismatch — a "liker" is semantically different from an "author." Consider aligning the DB column name toliker_id(with a migration) or renaming the JS field toauthor_idfor consistency.backend/eslint.config.js (2)
19-19: Remove or fix the commented-out rule — it contains a typo (newlice→newline).Stale commented-out code with a typo adds confusion. Either remove it or correct and enable it.
28-28: Emptyplugins: {}is unnecessary and can be removed.ui/src/lib/types.ts (1)
8-21: Inconsistent property naming convention — mix ofsnake_caseandcamelCase.
preview_image,preview_text,author_iduse snake_case whilelikesCount,createdAt,updatedAtuse camelCase. Pick one convention (typically camelCase for TypeScript) and transform at the API boundary.ui/src/components/article/article-more-button.tsx (2)
34-48: Placeholder handlers — follow, report, and delete actions areconsole.logstubs.These will need actual implementations. Consider tracking with TODO comments or issues.
Would you like me to open issues to track implementing these actions?
32-32: Component is tightly coupled to route/_main/article/$slugviauseParams.The
slugis only used for the edit navigation. Consider passingslugas a prop instead, which would make this component reusable outside this specific route.ui/src/components/write/publish-button.tsx (1)
140-153: Duplicated dynamic-height logic betweenonInputandonFocushandlers.The same height-auto-then-scrollHeight calculation appears in both handlers. Extract to a helper to reduce duplication.
Proposed refactor
+ const autoResize = (target: HTMLTextAreaElement, maxHeight = 120) => { + target.style.height = 'auto' + target.style.height = `${Math.min(target.scrollHeight, maxHeight)}px` + } + <Textarea value={previewTitle} onChange={(e) => setPreviewTitle(e.target.value)} - onInput={(e) => { - const target = e.target as HTMLTextAreaElement - target.style.height = 'auto' - target.style.height = `${Math.min(target.scrollHeight, 120)}px` - }} - onFocus={(e) => { - const target = e.target as HTMLTextAreaElement - target.style.height = 'auto' - target.style.height = `${Math.min(target.scrollHeight, 120)}px` - }} + onInput={(e) => autoResize(e.target as HTMLTextAreaElement)} + onFocus={(e) => autoResize(e.target as HTMLTextAreaElement)} placeholder="Enter your article title" rows={1} className="text-lg min-h-9 resize-none overflow-hidden" />ui/src/data/queries/article.ts (1)
42-44: Hardcoded "You are not authorized" for all like errors is misleading.Network failures, server errors, and rate limits will all show an auth error message. Consider either inspecting the error status/type or using a generic message like
'Failed to update like'.ui/src/components/write/update-button.tsx (1)
16-36: Significant duplication withPublishButton.The validation schema, dialog layout, state management pattern, and image upload flow closely mirror
PublishButton.tsx. If these diverge over time, bugs fixed in one won't be fixed in the other. Consider extracting a sharedArticleMetadataDialogcomponent or at least a shared schema/validation utility when convenient.Also applies to: 56-92
ui/src/components/article/content-bar.tsx (1)
28-28: Like button has no guard against rapid clicks.
useToggleArticleLikedoesn't exposeisPending, so the button isn't disabled during the in-flight mutation. Rapid clicks will fire multiple toggle requests, potentially racing against each other and leaving the like state non-deterministic until the final invalidation settles. Consider destructuringisPendingand disabling the button while the mutation is in flight.💡 Suggested fix
- const { mutate: toggleLike } = useToggleArticleLike() + const { mutate: toggleLike, isPending: isToggling } = useToggleArticleLike()<Button onClick={() => toggleLike(articleID)} + disabled={isToggling} variant="link"Also applies to: 35-48
backend/src/services/article.service.ts (1)
110-158: GetPostBySlug:isLikedByUsercheck could be folded into the main query.The separate DB round-trip to check whether the user liked the article works correctly, but for high-traffic read paths this adds latency. Consider using a conditional left join or a correlated subquery in the main query to retrieve this in a single round-trip. Not blocking, but worth considering.
ui/src/components/article/safe-html-renderer.tsx (1)
19-33: Simplify: replace dualuseLayoutEffect(no deps) with a single one keyed onhtmlContent.Both
useLayoutEffecthooks run on every render because they have no dependency arrays. The version-tracking refs exist solely to deduplicate, but a single effect with[htmlContent]as the dependency achieves the same result more simply and avoids unnecessary work on unrelated re-renders.♻️ Proposed simplification
- const contentVersionRef = useRef(0) - const lastHighlightedVersionRef = useRef(-1) - - useLayoutEffect(() => { - contentVersionRef.current += 1 - }) - - useLayoutEffect(() => { - if ( - containerRef.current && - lastHighlightedVersionRef.current !== contentVersionRef.current - ) { - containerRef.current.querySelectorAll('pre code').forEach((block) => { - hljs.highlightElement(block as HTMLElement) - }) - lastHighlightedVersionRef.current = contentVersionRef.current - } - }) + useLayoutEffect(() => { + if (containerRef.current) { + containerRef.current.querySelectorAll('pre code').forEach((block) => { + hljs.highlightElement(block as HTMLElement) + }) + } + }, [htmlContent])ui/src/routes/article.edit.$slug.tsx (1)
8-21: Silent redirect on unauthorized access may confuse users.Both the unauthenticated case (Line 11) and the non-author case (Line 20) silently redirect to
/with no indication of why. Consider usingredirectwith a search param (e.g.,?error=unauthorized) or using a toast/flash mechanism so the user understands they were redirected.ui/src/routes/_main/article/$slug.tsx (1)
23-23: Misleading prop name:userpassed asauthor.The route context's
useris the currently authenticated user, not the article's author. Passing it asauthoris confusing sinceArticleViewalso accessesarticle.authorfor the actual author display. The prop is used for ContentBar'suserID(i.e., the viewer), so naming itcurrentUserorviewerinArticleViewPropswould be clearer.ui/src/components/article/article-view.tsx (1)
53-61: Consider extracting shared ContentBar props to reduce duplication.Both
ContentBarinstances receive identical props. A small variable would keep them in sync and reduce the risk of future drift.Example
+ const contentBarProps = { + likesCount: article.likesCount, + authorID: article.author_id, + articleTitle: article.title, + userID: author?.id, + onCommentClick: scrollToComments, + isLiked: article.isLikedByUser, + articleID: article.id, + } + ... - <ContentBar - likesCount={article.likesCount} - authorID={article.author_id} - ... - /> + <ContentBar {...contentBarProps} /> <SafeHtmlRenderer htmlContent={article.content} /> - <ContentBar - likesCount={article.likesCount} - ... - /> + <ContentBar {...contentBarProps} />Also applies to: 64-72
| /* Dark Mode Adjustments for Code */ | ||
| .dark .tiptap pre { | ||
| background-color: oklch(0.2 0 0); /* Slightly lighter than pure black */ | ||
| color: oklch(0.9 0 0); /* Off-white text */ | ||
|
|
||
| .hljs-strong { | ||
| font-weight: 700; | ||
| } | ||
| .hljs-keyword { | ||
| color: oklch(0.75 0.15 300); | ||
| } | ||
| .hljs-string { | ||
| color: oklch(0.75 0.15 150); | ||
| } | ||
| .hljs-title { | ||
| color: oklch(0.75 0.15 250); | ||
| } | ||
| .hljs-variable { | ||
| color: oklch(0.75 0.15 30); | ||
| } | ||
| } |
There was a problem hiding this comment.
Incomplete dark-mode overrides — several token groups will have poor contrast.
The dark-mode block only adjusts 4 token classes (.hljs-keyword, .hljs-string, .hljs-title, .hljs-variable), but the light-mode block defines 8 distinct color groups. The un-overridden groups retain their light-mode colors (lightness 0.45–0.6) against the dark oklch(0.2 0 0) background, which will produce poor contrast:
| Missing dark override | Light-mode lightness | Readability on dark bg |
|---|---|---|
.hljs-attr, .hljs-built_in, .hljs-type, .hljs-params |
0.50 | borderline |
.hljs-bullet, .hljs-symbol, .hljs-link, .hljs-meta, … |
0.45 | poor |
.hljs-number, .hljs-literal, .hljs-doctag, .hljs-regexp |
0.60 | marginal |
.hljs-addition, .hljs-selector-tag |
0.55 | marginal |
Add dark-mode overrides for all token groups, raising lightness to ~0.75 as done for the four already covered.
🎨 Proposed dark-mode additions
.dark .tiptap pre {
background-color: oklch(0.2 0 0);
color: oklch(0.9 0 0);
.hljs-keyword {
color: oklch(0.75 0.15 300);
}
.hljs-string {
color: oklch(0.75 0.15 150);
}
.hljs-title {
color: oklch(0.75 0.15 250);
}
.hljs-variable {
color: oklch(0.75 0.15 30);
}
+ .hljs-number,
+ .hljs-literal,
+ .hljs-doctag,
+ .hljs-regexp {
+ color: oklch(0.75 0.15 150);
+ }
+ .hljs-attr,
+ .hljs-built_in,
+ .hljs-type,
+ .hljs-params {
+ color: oklch(0.7 0.12 40);
+ }
+ .hljs-bullet,
+ .hljs-symbol,
+ .hljs-link,
+ .hljs-meta,
+ .hljs-selector-attr,
+ .hljs-selector-pseudo {
+ color: oklch(0.72 0.14 340);
+ }
+ .hljs-addition,
+ .hljs-selector-tag {
+ color: oklch(0.75 0.15 300);
+ }
+ .hljs-name,
+ .hljs-section,
+ .hljs-selector-id,
+ .hljs-selector-class {
+ color: oklch(0.75 0.15 250);
+ }
}📝 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.
| /* Dark Mode Adjustments for Code */ | |
| .dark .tiptap pre { | |
| background-color: oklch(0.2 0 0); /* Slightly lighter than pure black */ | |
| color: oklch(0.9 0 0); /* Off-white text */ | |
| .hljs-strong { | |
| font-weight: 700; | |
| } | |
| .hljs-keyword { | |
| color: oklch(0.75 0.15 300); | |
| } | |
| .hljs-string { | |
| color: oklch(0.75 0.15 150); | |
| } | |
| .hljs-title { | |
| color: oklch(0.75 0.15 250); | |
| } | |
| .hljs-variable { | |
| color: oklch(0.75 0.15 30); | |
| } | |
| } | |
| /* Dark Mode Adjustments for Code */ | |
| .dark .tiptap pre { | |
| background-color: oklch(0.2 0 0); /* Slightly lighter than pure black */ | |
| color: oklch(0.9 0 0); /* Off-white text */ | |
| .hljs-keyword { | |
| color: oklch(0.75 0.15 300); | |
| } | |
| .hljs-string { | |
| color: oklch(0.75 0.15 150); | |
| } | |
| .hljs-title { | |
| color: oklch(0.75 0.15 250); | |
| } | |
| .hljs-variable { | |
| color: oklch(0.75 0.15 30); | |
| } | |
| .hljs-number, | |
| .hljs-literal, | |
| .hljs-doctag, | |
| .hljs-regexp { | |
| color: oklch(0.75 0.15 150); | |
| } | |
| .hljs-attr, | |
| .hljs-built_in, | |
| .hljs-type, | |
| .hljs-params { | |
| color: oklch(0.7 0.12 40); | |
| } | |
| .hljs-bullet, | |
| .hljs-symbol, | |
| .hljs-link, | |
| .hljs-meta, | |
| .hljs-selector-attr, | |
| .hljs-selector-pseudo { | |
| color: oklch(0.72 0.14 340); | |
| } | |
| .hljs-addition, | |
| .hljs-selector-tag { | |
| color: oklch(0.75 0.15 300); | |
| } | |
| .hljs-name, | |
| .hljs-section, | |
| .hljs-selector-id, | |
| .hljs-selector-class { | |
| color: oklch(0.75 0.15 250); | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@ui/src/styles.css` around lines 283 - 300, Add dark-mode overrides for all
remaining highlight token selectors so their lightness is increased to ~0.75
against the dark pre background; specifically update the .dark .tiptap pre block
to include selectors such as .hljs-attr, .hljs-built_in, .hljs-type,
.hljs-params, .hljs-bullet, .hljs-symbol, .hljs-link, .hljs-meta, .hljs-number,
.hljs-literal, .hljs-doctag, .hljs-regexp, .hljs-addition, and
.hljs-selector-tag with oklch colors matching the existing pattern (e.g.,
oklch(0.75 0.15 <hue>)) so all token groups use higher lightness (~0.75) and
maintain consistent hues with the already defined
.hljs-keyword/.hljs-string/.hljs-title/.hljs-variable entries.
- Update `isomorphic-dompurify` to the latest version. - Enable `secure: true` for Cloudinary uploads. - Restrict `ALLOWED_URI_REGEXP` in `sanitize-html.ts` to `https?|mailto|tel|ftp`. - Correct the `useArticleEditor` hook to render when not SSR. - Use `slug` instead of `articleID` for sharing and article queries. - Refactor `QUERY_KEYS.GET_ARTICLE` to use a function for dynamic key generation. - Implement a more robust base URL determination for the API client. - Add client-side DOMPurify sanitization in `safe-html-renderer.tsx` as a fallback. - Update toasts to correctly refer to "article" instead of "story". - Remove redundant code block styling from `styles.css`.
1 participant
Summary by CodeRabbit
Release Notes
New Features
Style
Chores