-
Notifications
You must be signed in to change notification settings - Fork 6
Feature/configurable app #595
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
# Conflicts: # packages/renderer/dist/ecency-renderer.cjs # packages/renderer/dist/ecency-renderer.es.js # pnpm-lock.yaml
- Resolved SDK conflicts by keeping develop branch implementations - Updated self-hosted app to use getAccountPostsInfiniteQueryOptions - Kept all new SDK query options, types, and utilities from develop Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Accepts removal of packages/renderer (moved to apps/web/src/features/post-renderer) - Regenerates pnpm-lock.yaml with combined dependencies Co-Authored-By: Claude Opus 4.5 <[email protected]>
📝 WalkthroughWalkthroughAdds a new self-hosted React blog app under Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant AuthProvider
participant KeychainExt as "Keychain Extension"
participant HiveAPI as "Hive Network"
User->>Browser: Click "Login with Keychain"
Browser->>AuthProvider: login('keychain', username)
AuthProvider->>KeychainExt: handshake()
KeychainExt-->>AuthProvider: handshake OK
AuthProvider->>KeychainExt: signBuffer(account, challenge)
KeychainExt->>User: Prompt to approve signing
User->>KeychainExt: Approve
KeychainExt-->>AuthProvider: Signature
AuthProvider->>HiveAPI: Broadcast transaction
HiveAPI-->>AuthProvider: Confirmation
AuthProvider-->>Browser: Persist session, update context
sequenceDiagram
participant User
participant Browser
participant BlogPage
participant QueryClient
participant HiveAPI
User->>Browser: Navigate to post URL
Browser->>BlogPage: Render BlogPostPage
BlogPage->>QueryClient: fetch post (get_content)
QueryClient->>HiveAPI: get_content(author, permlink)
HiveAPI-->>QueryClient: Post data
QueryClient-->>BlogPage: Return post entry
BlogPage->>QueryClient: fetch discussions (get_discussion)
QueryClient->>HiveAPI: get_discussion(author, permlink)
HiveAPI-->>QueryClient: Comments list
QueryClient-->>BlogPage: Return discussions
BlogPage->>User: Render post and comments
Estimated Code Review Effort🎯 5 (Critical) | ⏱️ ~120 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
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧬 Code graph analysis (2)apps/self-hosted/src/features/blog/components/text-to-speech-button.tsx (1)
apps/self-hosted/src/features/auth/utils/hive-auth.ts (3)
⏰ 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). (1)
🔇 Additional comments (14)
✏️ Tip: You can disable this entire section by setting 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 11
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🤖 Fix all issues with AI agents
In `@apps/self-hosted/package.json`:
- Line 31: The package.json currently pins a platform-specific dependency
"@esbuild/darwin-arm64" which will fail installs on non-macOS-arm64 CI runners;
remove that explicit entry (or move it out of dependencies/devDependencies) and
rely on the cross-platform "esbuild" package or esbuild's optional
platform-specific packages managed automatically; update package.json by
deleting the "@esbuild/darwin-arm64" dependency entry and ensure "esbuild" is
present as a devDependency if needed (refer to the dependency name
"@esbuild/darwin-arm64" in package.json).
In `@apps/self-hosted/src/features/auth/auth-provider.tsx`:
- Around line 49-51: The isBlogOwner memo currently calls
blogOwner.toLowerCase() (and user.username.toLowerCase()) which will throw if
blogOwner or user/username is undefined; update the useMemo in isBlogOwner to
guard both values — e.g., coerce to a safe string or check existence before
calling toLowerCase — by referencing isBlogOwner, useMemo, user, and blogOwner
so the comparison uses (blogOwner ?? '') and (user?.username ?? '') or an
explicit existence check before lowercasing.
In `@apps/self-hosted/src/features/auth/components/hiveauth-login.tsx`:
- Around line 158-170: The QRCode component currently sends the sensitive
payload to api.qrserver.com; update the QRCode function to render the QR locally
instead of using an external URL: replace the external img approach in QRCode
with a client-side QR generator (e.g., the qrcode.react <QRCode> component or a
similar library) so the 32-byte encryption key and auth UUID never leave the
device, ensure you import and use the chosen local QR renderer within the QRCode
component, and remove any external network calls/urls so the raw payload stays
on-device.
In `@apps/self-hosted/src/features/auth/utils/hive-auth.ts`:
- Around line 274-282: The broadcastWithHiveAuth function currently starts a
2-minute setTimeout but does not clear that timer on success/failure; mirror the
fix from loginWithHiveAuth by capturing the timeout ID (e.g., const timer =
setTimeout(...)) and ensure clearTimeout(timer) is called in the cleanup path
and immediately after any resolve/reject or success callback paths inside
broadcastWithHiveAuth so the timeout is cancelled once the flow completes or
errors.
- Around line 190-198: The timeout created with setTimeout (5 * 60 * 1000) is
not cleared on successful or failed authentication and may fire later; update
the code to capture the timer ID (const authTimeout = setTimeout(...)) and
ensure clearTimeout(authTimeout) is called inside cleanup() and also immediately
before any resolve/reject paths (including the success handler and the existing
reject on timeout) so the timer cannot fire after the WebSocket is closed or
authentication completes; reference the existing cleanup(), ws.readyState check,
and callbacks.onError usage to locate where to clear the timeout.
In `@apps/self-hosted/src/features/blog/components/blog-posts-list.tsx`:
- Around line 35-45: The current overrides drop the helpers' built-in enabled
guards (!!username / !!communityId); fix by preserving them: call
getAccountPostsInfiniteQueryOptions(...) and
getCommunityPostsInfiniteQueryOptions(...) into local consts (e.g.,
accountOptions, communityOptions) and pass {...accountOptions, select:
(data)=>data.pages.flat(), enabled: accountOptions.enabled && !isCommunityMode}
for the account query and {...communityOptions, select:
(data)=>data.pages.flat(), enabled: communityOptions.enabled && isCommunityMode}
for the community query so the helpers' enabled checks (the
!!username/!!communityId guards) are kept.
In `@apps/self-hosted/src/features/floating-menu/components/config-editor.tsx`:
- Around line 152-189: The array textarea loses in-progress edits because it's
fully controlled by JSON.stringify(arrayValue) and only updates parent via
handleChange on valid JSON; fix by adding a local draft state (e.g., draftJson)
inside the component that is initialized from
arrayValue/JSON.stringify(arrayValue), update draftJson on every textarea
onChange, attempt JSON.parse there and only call handleChange(parsed) when parse
succeeds, and use a useEffect watching value/arrayValue to sync/reset draftJson
when the external value actually changes; keep identifiers: arrayValue,
handleChange, fullPath, field, value.
In `@apps/self-hosted/src/features/shared/user-avatar.tsx`:
- Around line 83-97: The clickable avatar uses a non-focusable <span> with
onClick, so make it keyboard-accessible by: when onClick is provided, set
role="button" and tabIndex={0} on the span, and add an onKeyDown handler that
triggers the same onClick for Enter and Space (prevent default for Space to
avoid page scroll); keep the existing aria-label and cursor-pointer logic.
Update the JSX rendered in user-avatar.tsx (the span returned in the component)
to conditionally add role/tabIndex/onKeyDown only when onClick exists so
non-clickable avatars remain unchanged.
In `@apps/self-hosted/src/routes/__root.tsx`:
- Around line 16-18: FloatingMenu is currently rendered unconditionally inside
AuthProvider (AuthProvider, Outlet, FloatingMenu) exposing the ConfigEditor to
unauthenticated visitors; fix by gating FloatingMenu behind the
authentication/authorization state provided by your auth context (e.g., import
and call useAuth or useUser from your AuthProvider) and only render
<FloatingMenu ...> when the user is authenticated (and optionally user.isAdmin
or isBlogOwner for admin-only access); also remove the unconditional show={true}
or set it based on the auth check so only authorized users see the editor.
In `@apps/self-hosted/src/routes/login.tsx`:
- Around line 11-16: validate the user-controlled redirect before passing to
navigate: update the existing validateSearch function (exported Route
createFileRoute block) to sanitize the redirect value and/or add a helper (e.g.,
isSafeRedirect or sanitizeRedirect) used by LoginPage when calling navigate so
only internal paths are allowed (relative paths starting with a single '/' and
not starting with '//' or containing a scheme like 'http:'), otherwise fall back
to '/' ; ensure both validateSearch and the LoginPage navigation points
reference this helper to prevent open-redirects.
In `@apps/self-hosted/src/styles/blog-markdown.css`:
- Around line 15-71: The dark/light CSS rules inside `@media`
(prefers-color-scheme: dark) incorrectly target .markdown-body and
[data-theme="dark"] together, preventing manual theme switching; refactor by
moving base .markdown-body rules outside the media queries, then add explicit
ancestor-scoped rules such as [data-theme="dark"] .markdown-body and
[data-theme="light"] .markdown-body for manual theme overrides, and keep `@media`
(prefers-color-scheme: dark) only for fallback/system-preference rules; update
the selectors in this file that reference .markdown-body and [data-theme="dark"]
so they follow this ancestor pattern instead of combining them inside the media
query.
🟡 Minor comments (21)
apps/self-hosted/src/styles/fonts.css-1-6 (1)
1-6: Document CSP requirements for Google Fonts if enforcing CSP headers.Google Fonts introduces an external CDN dependency for premium themes (magazine, developer, modern-gradient). While system font fallbacks are in place and the app functions without them, consider documenting required CSP directives (
fonts.googleapis.com,fonts.gstatic.com) if strict CSP is deployed, or making theme selection configurable to allow users to opt into system-only fonts.apps/self-hosted/package.json-23-24 (1)
23-24: Remove the unusedmotionpackage from dependencies.The
motionpackage (v12.23.22) is listed in package.json but is not imported anywhere in the codebase. Onlyframer-motion(v11.18.2) is actively used inapps/self-hosted/src/features/blog/components/blog-post-item.tsx. Remove the unusedmotiondependency to reduce bundle size and eliminate confusion.apps/self-hosted/src/features/floating-menu/utils.ts-16-39 (1)
16-39: Edge case: empty path string causes array index error.If
pathis an empty string,keyswill be['']andkeys[keys.length - 1]!will be an empty string, which could lead to unexpected behavior when setting the property.🛡️ Suggested defensive check
export function updateNestedPath( obj: Record<string, ConfigValue>, path: string, value: ConfigValue, ): Record<string, ConfigValue> { + if (!path) { + return obj; + } const newObj = deepClone(obj); const keys = path.split('.');apps/self-hosted/src/features/blog/layout/blog-sidebar.tsx-85-97 (1)
85-97: Validate and sanitize the website URL before rendering.User-provided URLs from
data.profile.websiteare rendered directly in an anchor tag. Whilerel="noopener noreferrer"mitigates some risks, consider validating the URL scheme to preventjavascript:URLs.🛡️ Suggested URL validation
+const isValidUrl = (url: string) => { + try { + const parsed = new URL(url); + return ['http:', 'https:'].includes(parsed.protocol); + } catch { + return false; + } +}; - {data?.profile?.website && ( + {data?.profile?.website && isValidUrl(data.profile.website) && ( <div className="text-xs text-theme-muted">apps/self-hosted/src/features/floating-menu/components/floating-menu-window.tsx-323-327 (1)
323-327:onKeyDownhandler on a non-focusable div may not receive keyboard events.The
onKeyDownhandler is attached to a div withpointer-events-none. Keyboard events require focus, and this div won't receive focus naturally. Consider addingtabIndexor attaching the handler to a focusable element.🔧 Suggested fix
<div - className="fixed inset-0 z-40 pointer-events-none" + className="fixed inset-0 z-40 pointer-events-none" + tabIndex={-1} + ref={(el) => el?.focus()} onKeyDown={handleKeyDown} >Alternatively, use a
useEffectto add a globalkeydownlistener:useEffect(() => { if (!isOpen) return; const handleEscape = (e: KeyboardEvent) => { if (e.key === 'Escape') onClose(); }; document.addEventListener('keydown', handleEscape); return () => document.removeEventListener('keydown', handleEscape); }, [isOpen, onClose]);apps/self-hosted/src/features/auth/components/keychain-login.tsx-19-19 (1)
19-19: Potential hydration mismatch due towindowcheck during render.
isKeychainAvailable()depends onwindow, which differs between server (wherewindowis undefined) and client. This causes the component to render different content on server vs client, leading to a React hydration mismatch warning.Consider using
useState+useEffectto check availability only on the client:Proposed fix
export function KeychainLogin({ onSuccess, onError }: KeychainLoginProps) { const { login } = useAuth(); const [username, setUsername] = useState(''); const [loading, setLoading] = useState(false); const [showInput, setShowInput] = useState(false); + const [keychainAvailable, setKeychainAvailable] = useState(false); - const keychainAvailable = typeof window !== 'undefined' && isKeychainAvailable(); + useEffect(() => { + setKeychainAvailable(isKeychainAvailable()); + }, []);apps/self-hosted/config.template.json-56-59 (1)
56-59: Typo:text2Speeechshould betext2Speech.The key has an extra 'e'. This will cause issues if code references the correct spelling.
Fix typo
"post": { - "text2Speeech": { + "text2Speech": { "enabled": true } },apps/self-hosted/src/features/blog/components/search-results.tsx-18-28 (1)
18-28: Guard against empty author scoping.
Ifusernameis unset in blog mode, the query becomesauthor:which can yield no results or API errors. Consider falling back to the base query (or a config error) whenusernameis empty.🛠️ Suggested guard
- // Search within blog author's posts - return `${baseQuery} author:${username} type:post`; + // Search within blog author's posts when configured + if (username) { + return `${baseQuery} author:${username} type:post`; + } + return baseQuery;apps/self-hosted/src/features/auth/components/hiveauth-login.tsx-48-69 (1)
48-69: Avoid double error reporting.The
statusdependency inhandleLogincreates a stale closure issue. When an error occurs, the event listener setsstatus='error'and callsonError, but thestatusvariable captured inhandleLogin's closure retains its old value ('idle'). When the promise rejects and the catch block runs, the checkif (status !== 'error')evaluates to true using the stale value, causingonErrorto fire twice.Use
useRefinstead of relying on state to track whether an error was already handled, since refs don't trigger closure updates and won't become stale during execution.The success timing is correct—
login('hiveauth', …)resolves only after the user approves on the mobile app (whenauth_ackis received).🔧 Suggested fix
-import { useCallback, useEffect, useState } from 'react'; +import { useCallback, useEffect, useRef, useState } from 'react'; export function HiveAuthLogin({ onSuccess, onError }: HiveAuthLoginProps) { const { login } = useAuth(); const [username, setUsername] = useState(''); const [loading, setLoading] = useState(false); const [showInput, setShowInput] = useState(false); const [qrData, setQrData] = useState<string | null>(null); const [status, setStatus] = useState<'idle' | 'waiting' | 'error'>('idle'); + const errorHandledRef = useRef(false); // Listen for HiveAuth events useEffect(() => { const handleQRCode = (e: CustomEvent<string>) => { setQrData(e.detail); setStatus('waiting'); }; const handleWaiting = () => { setStatus('waiting'); }; const handleError = (e: CustomEvent<string>) => { + errorHandledRef.current = true; setStatus('error'); onError?.(e.detail); setLoading(false); }; @@ setLoading(true); setStatus('idle'); setQrData(null); + errorHandledRef.current = false; try { await login('hiveauth', username.trim().toLowerCase()); onSuccess?.(); } catch (error) { // Error is handled by the event listener - if (status !== 'error') { + if (!errorHandledRef.current) { onError?.(error instanceof Error ? error.message : 'Login failed'); } } finally { setLoading(false); } - }, [username, login, onSuccess, onError, status]); + }, [username, login, onSuccess, onError]);apps/self-hosted/src/features/blog/components/text-to-speech-button.tsx-170-192 (1)
170-192: Chunking drops trailing text without terminal punctuation.The regex only captures sentences ending in punctuation, so a final sentence without punctuation is lost. Use a pattern that retains the trailing fragment.
🛠️ Suggested regex fix
-function splitIntoChunks(text: string, maxLength: number): string[] { - const sentences = text.match(/[^.!?]+[.!?]+/g) || [text]; +function splitIntoChunks(text: string, maxLength: number): string[] { + const sentences = + text.match(/[^.!?]+[.!?]*/g)?.filter(Boolean) || [text];apps/self-hosted/src/core/i18n.ts-390-406 (1)
390-406: Add a safe fallback ingetCurrentLanguage()to avoid undefined/unsupported values.Right now it returns the raw config value; if missing/invalid, callers can receive
undefined. Consider normalizing to a supported language (e.g.,'en').🛠️ Proposed fix
export function getCurrentLanguage(): string { - return InstanceConfigManager.getConfigValue( + const language = InstanceConfigManager.getConfigValue( ({ configuration }) => configuration.general.language, ); + return translations[language] ? language : 'en'; }apps/self-hosted/src/features/auth/components/comment-form.tsx-110-116 (1)
110-116: Add an accessible label for the textarea.Placeholders aren’t a substitute for labels. Adding an
aria-labelimproves screen reader usability.🛠️ Proposed fix
<textarea value={body} onChange={(e) => setBody(e.target.value)} placeholder={t('write_comment')} + aria-label={t('write_comment')} className="w-full px-3 py-2 rounded-md border border-theme bg-theme text-theme-primary placeholder:text-theme-muted focus:outline-none focus:ring-2 focus:ring-theme-strong resize-none" rows={3} disabled={isSubmitting} />apps/self-hosted/src/features/floating-menu/config-fields.ts-186-200 (1)
186-200: Fix typo intext2Speeechconfig key — consistently misspelled across codebase.The key
text2Speeechshould betext2Speech. While the misspelling is consistently used throughout the codebase (so it won't cause binding issues), it should still be corrected for code quality. Update the following locations:
apps/self-hosted/src/features/floating-menu/config-fields.ts:190apps/self-hosted/src/features/floating-menu/components/config-editor.tsx:28apps/self-hosted/src/features/blog/components/text-to-speech-button.tsx:22apps/self-hosted/config.template.json:57apps/self-hosted/src/features/blog/components/blog-post-discussion.tsx-106-153 (1)
106-153: Localize user-facing strings (loading, empty state, sort labels).These strings are currently hard-coded, which bypasses the i18n system used elsewhere. Consider moving them to
t(...)keys to support translations.apps/self-hosted/src/features/auth/components/vote-button.tsx-47-48 (1)
47-48: Count only positive votes when labeling as likes.
activeVotes.lengthwill include downvotes, but the UI label is “likes.” Consider filtering by positive weight/percent/rshares so the count matches the label.✅ Suggested adjustment
- const voteCount = activeVotes.length; + const voteCount = useMemo( + () => + activeVotes.filter((v) => { + const raw = v.weight ?? v.percent ?? v.rshares ?? 0; + const value = + typeof raw === 'string' ? Number.parseFloat(raw) : Number(raw); + return value > 0; + }).length, + [activeVotes] + );apps/self-hosted/src/features/blog/components/blog-post-discussion.tsx-69-76 (1)
69-76: Observer should be set to the current viewer, not the post author.Using
entryData.authorasobserverwill return vote/state data from the author's perspective instead of the logged-in viewer's. This means vote states and interaction flags will reflect the author's votes, not the current user's. Update this to use the authenticated user's username or an empty string for anonymous viewers.Add the import at the top:
import { useMemo, useState } from 'react'; +import { useAuth } from '@/features/auth';Then update the observer parameter (lines 69-76):
const response = await CONFIG.hiveClient.call( 'bridge', 'get_discussion', { author: entryData.author, permlink: entryData.permlink, - observer: entryData.author, + observer: user?.username ?? '', }, );Also add the hook call in the component body before the useQuery call:
export function BlogPostDiscussion({ entry, isRawContent }: Props) { + const { user } = useAuth(); const [order, setOrder] = useState<SortOrder>('created');apps/self-hosted/src/features/auth/auth-provider.tsx-109-137 (1)
109-137: HiveAuth login flow:isLoadingresets before async callbacks complete.The
finallyblock (line 146) setsisLoadingtofalseimmediately afterloginWithHiveAuthresolves, but the actual login completion happens asynchronously viaonSuccess. This means the loading state will befalsewhile still waiting for user interaction with the QR code.Consider managing loading state within the callbacks instead, or not setting
isLoadingtofalsefor the hiveauth case in the finally block.apps/self-hosted/src/features/auth/utils/hivesigner.ts-37-55 (1)
37-55: Handle potentialNaNfrom malformedexpires_inparameter.If
expires_inis present but contains a non-numeric string,parseIntwill returnNaN, which could cause issues downstream when calculating expiration times.Proposed fix
return { accessToken, username, - expiresIn: expiresIn ? parseInt(expiresIn, 10) : 604800, // Default 7 days + expiresIn: expiresIn && !isNaN(parseInt(expiresIn, 10)) + ? parseInt(expiresIn, 10) + : 604800, // Default 7 days };apps/self-hosted/src/features/auth/components/reblog-button.tsx-32-33 (1)
32-33: Consider case-insensitive comparison for Hive usernames.Hive usernames are case-insensitive. The comparison should normalize case to avoid false negatives.
Proposed fix
// Can't reblog your own post - const isOwnPost = user?.username === author; + const isOwnPost = user?.username?.toLowerCase() === author?.toLowerCase();apps/self-hosted/src/features/auth/utils/hive-auth.ts-51-61 (1)
51-61: Non-null assertion on regex match could cause runtime error.If
keyis an empty string or doesn't match the expected hex pattern,key.match(/.{2}/g)returnsnull, and the non-null assertion!will cause a runtime error. Also noted: the comment acknowledges this is a simplified XOR approach.Proposed defensive fix
function encryptChallenge(challenge: string, key: string): string { - const keyBytes = new Uint8Array(key.match(/.{2}/g)!.map((b) => parseInt(b, 16))); + const keyMatches = key.match(/.{2}/g); + if (!keyMatches || keyMatches.length === 0) { + throw new Error('Invalid encryption key format'); + } + const keyBytes = new Uint8Array(keyMatches.map((b) => parseInt(b, 16))); const challengeBytes = new TextEncoder().encode(challenge);apps/self-hosted/src/features/auth/utils/hivesigner.ts-60-73 (1)
60-73: Add error handling for the dynamic import and broadcast operation.The function lacks protection against failures from both the dynamic import and the
client.broadcast()call. If the hivesigner package is unavailable or the broadcast operation fails, errors will propagate without context. Add try-catch around the import and broadcast operation, or at minimum provide meaningful error messages to help with debugging.
🧹 Nitpick comments (34)
packages/sdk/src/modules/core/functions/rpcmessage.ts (2)
8-15: Use a separator when aggregating RPC info parts.Concatenating values with an empty delimiter can mash multiple messages together; a visible separator improves readability and debugging.
♻️ Suggested tweak
- return Object.values(jseInfo).join(""); + return Object.values(jseInfo).join(" ");
19-22: TightenisRpcErrorto avoid false positives.Presence-only checks can match any object with
name/messagekeys. Verifying their types makes the guard more reliable.♻️ Suggested tweak
export function isRpcError(err: unknown): err is RPCError { return ( - typeof err === "object" && err !== null && "message" in err && "name" in err + typeof err === "object" && + err !== null && + "message" in err && + "name" in err && + typeof (err as any).message === "string" && + typeof (err as any).name === "string" ); }packages/sdk/src/modules/posts/functions/validate-entry.ts (1)
63-71: Normalize partialstatsobjects, not just missing ones.If
entry.statsexists but is missing fields, defaults aren’t applied, leaving undefined access risks. Consider defaulting per field.♻️ Suggested refactor
- if (!entry.stats) { - entry.stats = { - flag_weight: 0, - gray: false, - hide: false, - total_votes: 0, - }; - } + entry.stats = { + ...(entry.stats ?? {}), + flag_weight: entry.stats?.flag_weight ?? 0, + gray: entry.stats?.gray ?? false, + hide: entry.stats?.hide ?? false, + total_votes: entry.stats?.total_votes ?? 0, + };apps/self-hosted/src/consts/react-query.ts (1)
1-3: QueryClient singleton is fine for SPA; verify SSR usage.If this app ever uses SSR/edge rendering, a module-level QueryClient can leak cache between requests—consider a per-request factory in that case.
apps/self-hosted/src/features/blog/queries/types.ts (1)
1-20: LGTM!The
Communityinterface is well-structured with appropriate optional fields.One optional enhancement: the
teamfield'sArray<Array<string>>type could be more self-documenting. If the inner array has a fixed structure (e.g.,[role, username, title]), consider a tuple type or a dedicated interface for clarity.// Example: if team entries have known structure type TeamMember = [role: string, username: string, title: string]; team: TeamMember[];apps/self-hosted/src/globals.css (1)
121-125: Fragile selector targeting Tailwind utility classes.The selector
.flex.items-center.gap-1targets specific Tailwind utility class combinations, which is brittle. If the component's classes change slightly, this style won't apply. Consider using a semantic class name instead.Suggested approach
- /* Increase tap area for small interactive elements */ - .flex.items-center.gap-1 { - padding: 4px 0; - } + /* Increase tap area for small interactive elements */ + .tap-target-sm { + padding: 4px 0; + }Then apply
.tap-target-smto the relevant components.apps/self-hosted/src/features/shared/error-message.tsx (1)
13-31: Consider using theme-aware colors for consistency.The component uses hardcoded Tailwind colors (
text-red-500,bg-blue-600,hover:bg-blue-700) while the rest of the app uses theme CSS variables (e.g.,text-theme-muted). For visual consistency across themes, consider using theme variables or defining semantic color classes.Example using theme-aware classes
- <UilExclamationTriangle className="w-12 h-12 text-red-500 mb-4" /> + <UilExclamationTriangle className="w-12 h-12 text-theme-error mb-4" /> <p className="text-theme-muted text-center mb-4"> {message || t('error_loading')} </p> {onRetry && ( <button type="button" onClick={onRetry} - className="px-4 py-2 bg-blue-600 text-white rounded-md hover:bg-blue-700 transition-colors text-sm" + className="px-4 py-2 bg-theme-accent text-white rounded-md hover:opacity-90 transition-colors text-sm" >This would require defining
--theme-errorand--theme-accentin your theme variables.apps/self-hosted/src/styles/blog-markdown.css (1)
595-597: Removing focus outline on.anchor:focusmay impact keyboard accessibility.Removing the outline on anchor focus without providing an alternative visible focus indicator can make it difficult for keyboard users to navigate. Consider keeping a visible focus state or using
:focus-visibleinstead.Suggested approach
.markdown-body .anchor:focus { - outline: none; + outline: none; +} + +.markdown-body .anchor:focus-visible { + outline: 2px solid var(--focus-outlineColor); + outline-offset: 2px; }apps/self-hosted/src/features/blog/layout/blog-sidebar.tsx (2)
18-24: Consider handling loading and error states for BlogSidebarContent.Unlike
CommunitySidebar, this component doesn't handle loading or error states from the query. If the account fetch fails or is slow, users may see incomplete content.♻️ Suggested enhancement
function BlogSidebarContent({ username }: { username: string }) { - const { data } = useQuery(getAccountFullQueryOptions(username)); + const { data, isLoading, isError } = useQuery(getAccountFullQueryOptions(username)); const joinDate = useMemo(() => { if (!data?.created) return null; return formatMonthYear(data.created); }, [data?.created]); + + if (isLoading) { + return ( + <div className="lg:sticky lg:top-0 border-b lg:border-b-0 lg:border-l border-theme p-4 sm:p-6 lg:h-screen lg:overflow-y-auto"> + <div className="animate-pulse"> + <div className="w-16 h-16 rounded-full bg-theme-tertiary mb-4" /> + <div className="h-4 w-32 bg-theme-tertiary rounded mb-2" /> + <div className="h-3 w-48 bg-theme-tertiary rounded" /> + </div> + </div> + ); + }
106-115: Consider extracting the image proxy URL logic to a shared utility.This pattern duplicates the proxy URL construction from
UserAvatar(seeapps/self-hosted/src/features/shared/user-avatar.tsxlines 68-75). Consider extracting to a shared utility for consistency.apps/self-hosted/src/features/floating-menu/components/floating-menu-window.tsx (1)
241-245: Effect applies preview on every config change - consider debouncing for performance.This effect triggers
applyPreviewConfigon every config update while in preview mode. For rapid edits (e.g., typing in text fields), this could cause excessive DOM manipulation.♻️ Optional: debounce preview application
useEffect(() => { if (!isPreviewMode) return; const timeoutId = setTimeout(() => { applyPreviewConfig(config); }, 100); return () => clearTimeout(timeoutId); }, [config, isPreviewMode]);apps/self-hosted/src/features/auth/types.ts (1)
5-12: Document security sensitivity of credential fields.
AuthUsercontains sensitive fields (accessToken,refreshToken,postingKey). Consider adding JSDoc comments to warn against logging or exposing these values.📝 Suggested documentation
+/** + * Represents an authenticated user. + * `@warning` Contains sensitive credentials - never log or expose these values. + */ export interface AuthUser { username: string; + /** OAuth access token - sensitive */ accessToken?: string; + /** OAuth refresh token - sensitive */ refreshToken?: string; + /** Hive posting key - highly sensitive */ postingKey?: string; loginType: AuthMethod; expiresAt?: number; }apps/self-hosted/src/features/blog/components/detect-bottom.tsx (1)
14-24: Consider adding a guard against repeatedonBottomcalls.The callback fires every time
isIntersectingbecomes true. If the parent doesn't immediately update state (e.g., during slow network requests), or if the user scrolls up and down,onBottommay fire multiple times before the previous load completes. This could trigger duplicate API calls.Consider using
{ once: true }behavior or tracking loading state:♻️ Option: disconnect after first intersection
const observer = new IntersectionObserver( (entries) => { const [entry] = entries; if (entry.isIntersecting) { onBottom(); + observer.disconnect(); } }, { threshold: 0.1, }, );Alternatively, ensure the parent component handles deduplication (e.g., checking
isLoadingbefore fetching).apps/self-hosted/src/routes/$category.$author.$permlink.tsx (1)
4-9: Consider extracting a sharedvalidateSearchhelper.
Line 6: This logic matches the/\$author/\$permlinkroute; a shared helper keeps them in sync and reduces duplication.apps/self-hosted/src/features/auth/components/keychain-login.tsx (1)
38-48: Consider addingnoopenerfor external link security.When opening external URLs, using
window.openwith just'_blank'may exposewindow.openerin older browsers. While modern browsers mitigate this, explicitly specifyingnoopeneris a good practice.Suggested fix
- onClick={() => window.open('https://hive-keychain.com/', '_blank')} + onClick={() => window.open('https://hive-keychain.com/', '_blank', 'noopener')}apps/self-hosted/src/routes/blog/route.tsx (1)
5-12: Consider validating thefilterparameter against allowed values.The
filteris cast directly fromsearch.filterwithout validation. WhileBlogPostsListhas a default fallback, invalid filter values could still propagate and cause unexpected behavior withcommunityFilterMap.Suggested improvement
+const VALID_FILTERS = ['posts', 'comments', 'replies', 'payout'] as const; +type BlogFilter = typeof VALID_FILTERS[number]; + export const Route = createFileRoute('/blog')({ component: RouteComponent, validateSearch: (search: Record<string, unknown>) => { + const filter = search.filter as string; return { - filter: (search.filter as string) || 'posts', + filter: VALID_FILTERS.includes(filter as BlogFilter) ? filter : 'posts', }; }, });apps/self-hosted/src/features/auth/components/user-menu.tsx (2)
21-30: Consider registering the click-outside listener only when the menu is open.The event listener is always active, even when the dropdown is closed. This is a minor inefficiency.
♻️ Optional optimization
useEffect(() => { + if (!isOpen) return; + const handleClickOutside = (event: MouseEvent) => { if (menuRef.current && !menuRef.current.contains(event.target as Node)) { setIsOpen(false); } }; document.addEventListener('mousedown', handleClickOutside); return () => document.removeEventListener('mousedown', handleClickOutside); - }, []); + }, [isOpen]);
62-81: Consider adding dropdown accessibility attributes.The dropdown menu button could benefit from
aria-haspopup="menu"and the dropdown content fromrole="menu"with keyboard navigation support (Escape to close). Not critical for initial implementation but improves screen reader UX.apps/self-hosted/src/features/auth/components/login-method-button.tsx (1)
41-64: Consider addingaria-hidden="true"to the spinner SVG.The loading spinner is decorative and should be hidden from screen readers. Additionally,
aria-busy={loading}on the button would communicate the loading state.♻️ Optional accessibility improvement
<button type="button" onClick={onClick} disabled={disabled || loading} + aria-busy={loading} className={clsx(<svg className="animate-spin h-5 w-5 text-theme-muted" xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" + aria-hidden="true" >apps/self-hosted/src/routes/$author.$permlink.tsx (1)
6-10: Inconsistent return type fromvalidateSearch.When
search.rawis undefined, returningundefinedfor the whole object creates an inconsistent shape. This may cause issues when consumingsearch.rawin the component. Consider returning a consistent object:Suggested fix
validateSearch: (search: Record<string, unknown>) => { return { - raw: search.raw !== undefined ? true : undefined, + raw: search.raw !== undefined, }; },Alternatively, if you need to distinguish "not provided" from "provided as false", use:
raw: search.raw !== undefined ? Boolean(search.raw) : undefined,apps/self-hosted/src/styles/themes/developer.css (1)
117-128: Blinking cursor may cause confusion or accessibility issues.The terminal cursor effect appends a blinking underscore to all
<pre>elements within.markdown-body. This could:
- Mislead users into thinking the code block is editable or has a cursor
- Cause distraction for users sensitive to animations
- Interfere with code copy operations (though
::aftercontent typically isn't copied)Consider either:
- Limiting this to an opt-in class (e.g.,
.terminal-style)- Respecting
prefers-reduced-motionOptional: Add reduced-motion support
`@keyframes` blink { 50% { opacity: 0; } } + +@media (prefers-reduced-motion: reduce) { + [data-style-template="developer"] .markdown-body pre::after { + animation: none; + } +}apps/self-hosted/src/features/auth/components/create-post-button.tsx (1)
16-20: Unsafe type casting bypasses TypeScript safety.The double cast
(configuration.general as Record<string, unknown>).createPostUrl as stringsuppresses type checking. IfcreatePostUrlis missing or not a string, this silently passes through.Consider a safer pattern:
Safer config access
const createPostUrl = InstanceConfigManager.getConfigValue( - ({ configuration }) => - (configuration.general as Record<string, unknown>).createPostUrl as string || - 'https://ecency.com/submit', + ({ configuration }) => { + const url = (configuration.general as Record<string, string | undefined>)?.createPostUrl; + return typeof url === 'string' && url ? url : 'https://ecency.com/submit'; + }, );Ideally, extend the configuration type definition to include
createPostUrl?: stringso casting isn't needed.apps/self-hosted/src/features/blog/hooks/use-instance-config.ts (1)
7-12: Harden instance type parsing.
The cast allows any truthy string, which can leave both modes false. Prefer explicit validation.♻️ Suggested refactor
export function useInstanceConfig() { - const type = InstanceConfigManager.getConfigValue( + const rawType = InstanceConfigManager.getConfigValue( ({ configuration }) => - (configuration.instanceConfiguration.type as InstanceType) || 'blog' + configuration.instanceConfiguration.type as string ); + const type: InstanceType = rawType === 'community' ? 'community' : 'blog';apps/self-hosted/src/index.tsx (2)
26-33: Guard against missing background config before splitting.If
backgroundis undefined, calling.split()will throw. Add a default and skip empty tokens.🛠️ Suggested defensive update
-InstanceConfigManager.getConfigValue( - ({ configuration }) => configuration.general.styles.background, -) - .split(' ') - .forEach((className) => { - document.body.classList.add(className); - }); +const backgroundClasses = + InstanceConfigManager.getConfigValue( + ({ configuration }) => configuration.general.styles.background ?? '', + ) || ''; +backgroundClasses + .split(' ') + .filter(Boolean) + .forEach((className) => { + document.body.classList.add(className); + });
159-168: Mount guard can skip rendering when markup already exists.If the root has SSR or placeholder content,
innerHTMLis non-empty and the app never mounts. Consider hydrating when markup exists or always mounting.🔁 Suggested hydration-aware mount
-const rootElement = document.getElementById('root')!; -if (!rootElement.innerHTML) { - const root = ReactDOM.createRoot(rootElement); - root.render( - <StrictMode> - <RouterProvider router={router} /> - </StrictMode>, - ); -} +const rootElement = document.getElementById('root')!; +const app = ( + <StrictMode> + <RouterProvider router={router} /> + </StrictMode> +); +if (rootElement.hasChildNodes()) { + ReactDOM.hydrateRoot(rootElement, app); +} else { + ReactDOM.createRoot(rootElement).render(app); +}apps/self-hosted/src/features/auth/constants.ts (1)
6-14: Consider making OAuth/HiveAuth endpoints configurable for self-hosted installs.Hard-coded
HIVESIGNER_*andHIVEAUTH_*endpoints can limit private deployments. Exposing these via instance config or env with defaults would improve portability.apps/self-hosted/src/styles/components.css (1)
216-230: Prefer:focus-visiblefor focus rings (keyboard-friendly).Using
:focusshows rings on mouse clicks;:focus-visibleis a better default for accessibility.✅ Suggested tweak
-.input-theme:focus { +.input-theme:focus-visible { outline: none; border-color: var(--theme-accent); box-shadow: 0 0 0 2px rgba(59, 130, 246, 0.3); } -.focus-ring-theme:focus { +.focus-ring-theme:focus-visible { outline: none; box-shadow: 0 0 0 2px var(--theme-accent); }apps/self-hosted/src/core/date-formatter.ts (1)
44-50: Consider handling additional format token differences.The conversion handles year (
YYYY→yyyy) and day (DD→dd), but other common tokens likehh(12-hour) vsHH(24-hour) or month tokens may also differ between format conventions. The current implementation should work for the documented config formats, but consider documenting which input formats are supported.apps/self-hosted/src/features/auth/components/reblog-button.tsx (1)
111-119: Hardcoded English strings should uset()for i18n consistency.The button title strings are hardcoded in English while the component uses
t()elsewhere. Apply the translation function for consistency.Proposed fix
title={ !isAuthenticated - ? 'Login to reblog' + ? t('login_to_reblog') : isOwnPost - ? "You can't reblog your own post" + ? t('cannot_reblog_own_post') : hasReblogged - ? 'Already reblogged' - : 'Reblog to your followers' + ? t('already_reblogged') + : t('reblog_to_followers') }apps/self-hosted/src/features/auth/storage.ts (1)
56-60: Consider documenting the permissive behavior for missingexpiresAt.Returning
truewhenexpiresAtis missing is a reasonable fallback for legacy or externally-created sessions, but this permissive behavior could lead to sessions that never expire ifsaveUserfails to setexpiresAt. Consider adding a brief inline comment explaining this design choice.apps/self-hosted/src/core/configuration-loader.ts (2)
16-17:as constprovides type-level immutability only, not runtime freezing.The spread operator creates a shallow copy with readonly types, but doesn't prevent runtime mutations. If runtime immutability is desired, consider using
Object.freeze().💡 Suggested change for true immutability
- export const CONFIG = { ...config } as const; + export const CONFIG = Object.freeze({ ...config }) as const;
38-40:useMemodependency on function reference may not provide expected memoization.If
conditionis a new arrow function on each render (e.g.,condition={(c) => c.someValue}), the memoization will be ineffective. Consider documenting that callers should useuseCallbackfor stable function references, or evaluate the condition outside the hook.apps/self-hosted/src/features/auth/utils/keychain.ts (2)
125-127: Redundant success check aftersignBuffer.The
signBufferfunction already rejects with an error when!response.success(lines 68-70), so this check will never be true. Consider removing it to simplify the code.♻️ Suggested simplification
export async function loginWithKeychain(username: string): Promise<string> { // First handshake await handshake(); // Create a challenge message const challenge = `Login to Ecency Blog: ${Date.now()}`; // Sign the challenge - const response = await signBuffer(username, challenge, 'Posting'); - - if (!response.success) { - throw new Error('Failed to sign login challenge'); - } + await signBuffer(username, challenge, 'Posting'); return username; }
40-51: Add timeout handling to handshake to prevent indefinite hangs.The
requestHandshakecallback has no error parameter and won't fire if the extension fails or the user closes the popup, leaving the promise unresolved indefinitely. Consider adding a timeout similar to thebroadcast()function (which uses a 30-second timeout) to gracefully handle cases where the handshake never completes.
apps/self-hosted/src/features/auth/components/hiveauth-login.tsx
Outdated
Show resolved
Hide resolved
apps/self-hosted/src/features/floating-menu/components/config-editor.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/self-hosted/src/features/auth/utils/hive-auth.ts`:
- Around line 47-61: The current encryptChallenge function uses XOR which is
incompatible with HiveAuth; replace its body to perform AES-CBC with PKCS7
padding and EVP_BytesToKey key derivation: in encryptChallenge(challenge:
string, key: string) derive a 32-byte AES key and 16-byte IV from the provided
key string using EVP_BytesToKey (MD5-based KDF used by HiveAuth), then encrypt
the UTF-8 challenge with AES-256-CBC + PKCS7 padding and return a base64-encoded
ciphertext; you can implement this using CryptoJS.AES.encrypt (ensuring you
supply the derived key and IV) or the Web Crypto API (derive key/iv, importKey,
and encrypt), and ensure the final output matches HiveAuth’s base64 format.
In `@apps/self-hosted/src/features/floating-menu/components/config-editor.tsx`:
- Line 28: Fix the typo in the icon key by renaming the object property
text2Speeech to text2Speech in the config mapping inside config-editor.tsx, and
update any places that reference the old key (text2Speeech) to use text2Speech
so icon lookup works correctly.
♻️ Duplicate comments (1)
apps/self-hosted/src/features/floating-menu/components/config-editor.tsx (1)
231-242: Previous review concern addressed.The array field now correctly delegates to
ArrayFieldEditorwhich maintains draft state to preserve intermediate edits.
🧹 Nitpick comments (2)
apps/self-hosted/package.json (1)
23-24: Remove redundantframer-motiondependency.
motion(v12+) is the renamed successor toframer-motion. Since you're already usingmotionv12, removeframer-motionto reduce bundle size and avoid maintaining two versions of the same library.Note: The codebase shows no direct imports of either package. Verify that
motionis being used (possibly transitively) before removingframer-motion, or consider if either dependency can be removed entirely.Proposed fix
"date-fns-tz": "^3.2.0", - "framer-motion": "^11.18.2", "motion": "^12.23.22",apps/self-hosted/src/features/floating-menu/components/config-editor.tsx (1)
200-229: Consider handling NaN for robustness.When the user pastes or enters invalid numeric text that bypasses browser validation,
Number(e.target.value)producesNaN. This could propagate to the config.Optional defensive handling
onChange={(e) => - handleChange( - e.target.value === '' ? null : Number(e.target.value), - ) + { + const num = Number(e.target.value); + handleChange( + e.target.value === '' ? null : (Number.isNaN(num) ? null : num), + ); + } }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/self-hosted/package.jsonapps/self-hosted/src/features/auth/auth-provider.tsxapps/self-hosted/src/features/auth/components/hiveauth-login.tsxapps/self-hosted/src/features/auth/utils/hive-auth.tsapps/self-hosted/src/features/blog/components/blog-posts-list.tsxapps/self-hosted/src/features/floating-menu/components/config-editor.tsxapps/self-hosted/src/features/shared/user-avatar.tsxapps/self-hosted/src/routes/__root.tsxapps/self-hosted/src/routes/login.tsxapps/self-hosted/src/styles/blog-markdown.css
🚧 Files skipped from review as they are similar to previous changes (7)
- apps/self-hosted/src/routes/__root.tsx
- apps/self-hosted/src/styles/blog-markdown.css
- apps/self-hosted/src/routes/login.tsx
- apps/self-hosted/src/features/auth/components/hiveauth-login.tsx
- apps/self-hosted/src/features/auth/auth-provider.tsx
- apps/self-hosted/src/features/shared/user-avatar.tsx
- apps/self-hosted/src/features/blog/components/blog-posts-list.tsx
🔇 Additional comments (12)
apps/self-hosted/package.json (2)
1-12: LGTM!Package metadata and scripts are well-structured. The
"private": trueis appropriate for an app package, and the ESM module type aligns with modern tooling conventions.
37-41: All dependency versions are valid and exist on npm. No action required.Verification confirms:
@types/nodev24.6.0 ✓ existstypescriptv5.9.3 ✓ exists (latest)@types/reactv19.1.16 ✓ exists (part of React 19 ecosystem)Likely an incorrect or invalid review comment.
apps/self-hosted/src/features/floating-menu/components/config-editor.tsx (7)
1-8: LGTM!Imports are well-organized, separating React hooks from local type and constant imports.
31-36: LGTM!The case-insensitive icon lookup with a sensible fallback is well implemented.
48-115: Well-implemented draft state pattern addressing the previous review concern.The
ArrayFieldEditorproperly maintains local draft state for intermediate edits while only propagating valid JSON to the parent. Good accessibility witharia-invalidfor validation feedback.
159-198: LGTM!Boolean toggle implementation is clean with proper accessibility: screen-reader-only input with descriptive aria-label and the visual toggle properly marked with
aria-hidden.
244-284: LGTM!The select field implementation with custom styling for the dropdown arrow is well-done.
286-316: LGTM!Default text field handling is straightforward, and setting
displayNameon the memoized component aids debugging in React DevTools.
318-378: LGTM!The
ConfigEditorcleanly separates sections from regular fields, usesuseMemoappropriately for the field categorization, and implements a responsive grid layout. The path propagation for nested config updates is correctly handled.apps/self-hosted/src/features/auth/utils/hive-auth.ts (3)
213-305: Sign flow lifecycle handling looks solid.Timeout cleanup and error handling are consistent and easy to follow.
310-312: Session validity check is clear and correct.
108-115: Unable to verify review comment without code inspectionThe HiveAuth protocol documentation reveals a potential discrepancy with the review's premise: according to the protocol specification, QR code data (auth_payload) contains only
account,uuid,key, andhost—it does not carry challenge fields. The challenge is instead sent encrypted within theauth_reqmessage itself, as part ofauth_req_data.If the implementation follows standard HiveAuth:
- QR generation should not include challenge data
- Challenge is transmitted separately, encrypted in
auth_req- The issue described (QR challenge diverging from
auth_reqchallenge) would not existIf the code implements a custom variant that places challenge data in QR, the suggested fix (persisting and reusing the same challenge/nonce) would violate the HiveAuth requirement that "each challenge request must use a fresh nonce" and "do not reuse nonces within the same session."
The repository could not be cloned for direct code inspection; manual verification of the
generateQRData()function implementation is required to determine whether the review comment's concern is valid or based on a protocol misunderstanding.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
apps/self-hosted/src/features/floating-menu/components/config-editor.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@apps/self-hosted/src/features/auth/utils/hive-auth.ts`:
- Around line 103-141: The issue is that two different HiveAuthChallenge objects
are created with separate Date.now() values, causing the encrypted auth_req and
the QR payload to contain different timestamps; fix this by creating the initial
challenge once (e.g., store the challenge object or its challenge string in an
outer-scope variable when preparing authReq) and reuse that same challenge when
calling generateQRData inside ws.onmessage (replace the second creation of a new
challenge with a reference to the original challenge used for encryptChallenge);
update references to use that stored challenge (variables: challenge, authReq,
encryptChallenge, generateQRData, uuid, username, key) so the QR and auth_req
contain identical challenge data.
- Around line 186-192: The ws.onclose handler currently only clears authTimeout
and leaves the authentication Promise unresolved; update the handler in the
function that establishes Hive auth (and apply the same pattern to
broadcastWithHiveAuth) to reject the pending Promise with an Error when the
socket closes unexpectedly: inside ws.onclose, after clearing authTimeout, call
the Promise's reject callback (e.g., reject(new Error("WebSocket closed before
authentication completed"))) and null out any resolve/reject refs to avoid
memory leaks so callers waiting on the auth Promise receive a timely error.
In `@apps/self-hosted/src/features/blog/components/text-to-speech-button.tsx`:
- Around line 134-141: The stop buttons in the TextToSpeechButton component are
icon-only and currently rely on title for accessible naming; update the button
elements (the ones using handleStop and rendering <UilStopCircle />) to include
an explicit aria-label (e.g., aria-label={t('stop')}) so screen readers get a
reliable accessible name; apply the same change to both stop button instances in
this component.
- Around line 171-189: The splitIntoChunks function drops trailing text without
terminal punctuation and fails when a single sentence exceeds maxLength; update
splitIntoChunks to (1) use a regex that also captures the final fragment without
punctuation (or fallback to treating remainingText as a sentence) so the last
fragment isn't dropped, and (2) when a sentence itself is longer than maxLength,
further split that sentence into sub-chunks of maxLength (respecting word
boundaries if possible) and push those sub-chunks into chunks; keep using
currentChunk to aggregate sentences, trim entries before pushing, and ensure the
final currentChunk is always pushed.
In `@apps/self-hosted/src/features/floating-menu/components/config-editor.tsx`:
- Around line 102-105: The style uses inputStyle.border (a full shorthand like
"1px solid rgba(...)") for borderColor which is incorrect; update
ArrayFieldEditor in config-editor.tsx to supply a proper color value instead of
the shorthand—either import/reference FLOATING_MENU_THEME to use its border
color (e.g., FLOATING_MENU_THEME.borderColor or equivalent), add a separate
inputStyle.borderColor property, or pass the color in as a prop and use that for
borderColor so the style assignment uses a pure color string rather than the
full border shorthand.
🧹 Nitpick comments (3)
apps/self-hosted/package.json (1)
24-25: Duplicate animation libraries detected.Both
framer-motionandmotionare included. Since version 11.11, Framer Motion was rebranded tomotion. Having both increases bundle size unnecessarily. Choose one:
- Keep
motion(newer, recommended going forward)- Keep
framer-motion(if you need pre-11.11 compatibility)Suggested fix (keep motion only)
"date-fns-tz": "^3.2.0", - "framer-motion": "^11.18.2", "motion": "^12.23.22",apps/self-hosted/src/features/auth/utils/hive-auth.ts (1)
227-241: Hardcodedkey_type: 'posting'limits operation flexibility.The signing request always uses
'posting'key type. If this module needs to support operations requiring active key authority (like transfers or witness operations), this would need modification.Consider making
key_typeconfigurable if active key operations are anticipated:export async function broadcastWithHiveAuth( session: HiveAuthSession, operations: Operation[], - callbacks?: HiveAuthSignCallback + callbacks?: HiveAuthSignCallback, + keyType: 'posting' | 'active' = 'posting' ): Promise<void> { // ... const signReq = { cmd: 'sign_req', account: session.username, token: session.token, data: { - key_type: 'posting', + key_type: keyType, ops: operations, broadcast: true, }, };apps/self-hosted/src/features/floating-menu/components/config-editor.tsx (1)
244-284: Consider adding a placeholder option for select fields.If
selectValueis an empty string and no option has an empty value, the select will display the first option, but the controlled value won't match. This could cause confusing UX. Consider adding a disabled placeholder option.Example placeholder option
<select ... > + <option value="" disabled> + Select an option... + </option> {options.map((option) => ( <option key={option.value} value={option.value}> {option.label} </option> ))} </select>
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
apps/self-hosted/config.template.jsonapps/self-hosted/package.jsonapps/self-hosted/src/features/auth/utils/hive-auth.tsapps/self-hosted/src/features/blog/components/text-to-speech-button.tsxapps/self-hosted/src/features/floating-menu/components/config-editor.tsxapps/self-hosted/src/features/floating-menu/config-fields.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/self-hosted/src/features/floating-menu/config-fields.ts
🧰 Additional context used
🧬 Code graph analysis (2)
apps/self-hosted/src/features/floating-menu/components/config-editor.tsx (4)
apps/self-hosted/src/features/floating-menu/config-fields.ts (1)
ConfigField(9-15)apps/self-hosted/src/features/floating-menu/types.ts (3)
ConfigValue(8-8)ConfigFieldEditorProps(17-23)ConfigEditorProps(10-15)apps/self-hosted/src/features/floating-menu/constants.ts (1)
FLOATING_MENU_THEME(2-10)apps/self-hosted/src/features/floating-menu/components/index.ts (1)
ConfigEditor(1-1)
apps/self-hosted/src/features/blog/components/text-to-speech-button.tsx (1)
apps/self-hosted/src/core/i18n.ts (1)
t(390-397)
⏰ 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). (1)
- GitHub Check: build (20.x)
🔇 Additional comments (14)
apps/self-hosted/config.template.json (1)
1-68: Looks good as a comprehensive default template.Clear, complete baseline config for the self-hosted app.
apps/self-hosted/package.json (3)
1-12: LGTM!Package metadata and scripts are well-configured for an rsbuild-based React application with biome for linting/formatting.
31-43: No action needed—TypeScript 5.9.3 is the current latest stable version.The specified version is valid and is the current stable release available on npm.
29-29: No peer dependency warnings expected with React 19.
react-usev17.6.0 declares support for any React version via wildcard peer dependencies ("react": "*"), so no peer dependency warnings will occur with React 19. If React 19 compatibility is a concern, verify through testing rather than peer dependency checks.Likely an incorrect or invalid review comment.
apps/self-hosted/src/features/auth/utils/hive-auth.ts (1)
302-308: LGTM!Session validation logic correctly handles null checks and properly converts the expire timestamp from seconds to milliseconds for comparison.
apps/self-hosted/src/features/floating-menu/components/config-editor.tsx (9)
1-8: LGTM!Imports are well-organized with proper separation of type imports using the
typekeyword.
10-29: LGTM!The section icons mapping is well-structured with consistent camelCase keys and proper type narrowing using
as const.
31-36: LGTM!The function correctly normalizes labels for case-insensitive matching and provides a sensible default fallback.
38-46: LGTM!Props interface is well-defined and provides clear typing for the ArrayFieldEditor component.
84-115: Good implementation of the ArrayFieldEditor component.The draft state pattern correctly preserves intermediate edits while only propagating valid JSON to the parent. Accessibility is well-handled with proper label associations and ARIA attributes.
160-198: LGTM!The boolean toggle switch is well-implemented with proper accessibility (sr-only input, descriptive aria-label) and consistent theming.
286-311: LGTM!The default text field implementation is clean with proper label association and consistent styling.
318-378: LGTM!The ConfigEditor component is well-structured with:
- Efficient separation of sections vs regular fields using
useMemo- Responsive grid layout for regular fields
- Proper key props for list rendering
- Safe optional chaining for config access
219-223: No action required.nullis explicitly included inConfigPrimitive(line 3 of types.ts) and is a validConfigValue, so the number field correctly passesnullwhen cleared.Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.