fix: improve remote user profile display and back navigation#133
fix: improve remote user profile display and back navigation#133
Conversation
- Add host domain to username display for remote users (@user@host format) - Add remote instance badge with icon, name, and theme color (matching NoteCard style) - Fix back button navigation using router.back() instead of window.history.back() This ensures proper SPA navigation and component re-rendering The instance badge links to the remote instance and shows: - Instance icon (with Globe fallback on error) - Instance name (or hostname as fallback) - Theme color styling when available
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughユーザープロファイルにリモートインスタンス統合を追加。ユーザーの Changes
Sequence Diagram(s)sequenceDiagram
participant UI as UserProfile (client)
participant API as getRemoteInstanceInfo()
participant Remote as Remote Instance Service
UI->>API: getRemoteInstanceInfo(host)
API->>Remote: HTTP fetch instance metadata
Remote-->>API: instance data (name, iconUrl, themeColor, softwareName)
API-->>UI: PublicRemoteInstance
alt remoteInstance.iconUrl exists
UI->>UI: render <img src=iconUrl loading="lazy">
alt image load fails
UI->>UI: set remoteInstanceIconFailed = true
UI->>UI: render Globe icon (fallback)
end
else no iconUrl
UI->>UI: render Globe icon
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@packages/frontend/src/components/user/UserProfile.tsx`:
- Around line 212-220: When switching between remote users the effect that runs
on user?.host change only resets remoteInstanceIconFailed in the else branch
(when host is falsy), so a previous failure state can carry over to a new remote
user; update the useEffect that references getRemoteInstanceInfo to proactively
reset remoteInstanceIconFailed (and optionally clear remoteInstance) before
starting the fetch: inside the truthy branch where
getRemoteInstanceInfo(user.host).then(setRemoteInstance) is called, call
setRemoteInstanceIconFailed(false) (and/or setRemoteInstance(null) immediately)
so each new host starts with a fresh, non-failed icon state.
- Around line 538-545: The inline use of remoteInstance.themeColor inside the
style prop in UserProfile.tsx risks CSS injection; update the code that builds
the style object (the backgroundColor and borderLeft values) to
validate/sanitize remoteInstance.themeColor before using it — for example, allow
only a safe CSS color format (hex, rgb/rgba, or a small whitelist) via a helper
like isValidColor(value) and fallback to "var(--bg-tertiary)" / "3px solid
var(--border-color)" when invalid or absent; ensure all references to
remoteInstance?.themeColor are replaced with the validated/sanitized value so no
raw remote string is inserted into CSS.
- Reset remoteInstance and remoteInstanceIconFailed states when switching between remote users to prevent stale state carry-over - Add validateCssColor() helper to sanitize themeColor before CSS injection Only allows safe color formats: hex, rgb/rgba, hsl/hsla
CodeRabbit レビューへの対応1. remoteInstanceIconFailed のリセット問題修正しました。 useEffect(() => {
if (user?.host) {
// Reset state before fetching new instance info
setRemoteInstance(null);
setRemoteInstanceIconFailed(false);
getRemoteInstanceInfo(user.host).then(setRemoteInstance);
} else {
setRemoteInstance(null);
setRemoteInstanceIconFailed(false);
}
}, [user?.host]);2. CSS インジェクションリスク修正しました。 function validateCssColor(color: string | null | undefined): string | null {
if (\!color || typeof color \!== "string") return null;
const trimmed = color.trim();
// Hex color: #RGB, #RRGGBB, #RGBA, #RRGGBBAA
if (/^#(?:[0-9a-f]{3}|[0-9a-f]{4}|[0-9a-f]{6}|[0-9a-f]{8})$/i.test(trimmed)) {
return trimmed;
}
// RGB/RGBA
if (/^rgba?\(\s*[\d.%,\s/]+\s*\)$/i.test(trimmed)) {
return trimmed;
}
// HSL/HSLA
if (/^hsla?\(\s*[\d.%,\s/deg]+\s*\)$/i.test(trimmed)) {
return trimmed;
}
return null; // Invalid or potentially dangerous
}不正な値の場合はフォールバック値( |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@packages/frontend/src/components/user/UserProfile.tsx`:
- Around line 245-256: When fetching remote instance info in the useEffect that
depends on user?.host, prevent race conditions by introducing a cancellation
flag (or AbortController) so that when getRemoteInstanceInfo(user.host) resolves
it first checks the flag before calling setRemoteInstance or
setRemoteInstanceIconFailed; ensure you set the flag to cancel in the effect
cleanup and only update state when not cancelled, using the existing identifiers
useEffect, user?.host, getRemoteInstanceInfo, setRemoteInstance and
setRemoteInstanceIconFailed.
- Around line 563-605: In UserProfile, the inline style uses
`${validatedThemeColor}15` which only works for hex colors; update the rendering
logic around validatedThemeColor (used in the backgroundColor and borderLeft in
the remote instance badge) to detect hex colors (e.g., via an isHexColor regex
or helper) and only append "15" for hex values, while for non-hex colors
(rgb()/hsl()) either convert them to an rgba() with alpha or fall back to
"var(--bg-tertiary)"; adjust the style computation inside the anonymous render
block that reads remoteInstance?.themeColor / validatedThemeColor so borderLeft
still uses the original color but backgroundColor uses the hex-alpha or the
converted rgba/fallback.
🧹 Nitpick comments (1)
packages/frontend/src/components/user/UserProfile.tsx (1)
429-443: バックボタンは React Aria/Button への統一を推奨します。
アクセシビリティ一貫性のため、native button ではなく既存の Button(React Aria ベース)に寄せたいです。♻️ 置き換え案
- <button - type="button" - onClick={() => { + <Button + variant="ghost" + onPress={() => { if (typeof window !== "undefined" && window.history.length > 1) { router.back(); } else { router.push("/timeline"); } }} className="absolute top-3 left-3 z-10 flex items-center gap-1 px-3 py-1.5 bg-black/40 hover:bg-black/60 text-white rounded-full text-sm font-medium transition-colors backdrop-blur-sm" aria-label="Go back" > <ArrowLeft className="w-4 h-4" /> <span className="hidden sm:inline"><Trans>Back</Trans></span> - </button> + </Button>コーディングガイドラインに基づき、React Aria の Button(または既存 Button)への統一を推奨します。
- Add cancellation flag to useEffect to prevent race conditions when switching between remote users quickly - Fix background color for non-hex theme colors: only append alpha suffix for hex colors, use fallback for rgb/hsl
追加の CodeRabbit レビューへの対応1. 非同期取得の競合問題キャンセルフラグを追加し、古いフェッチ結果が新しい状態を上書きしないようにしました: useEffect(() => {
let cancelled = false;
if (user?.host) {
setRemoteInstance(null);
setRemoteInstanceIconFailed(false);
getRemoteInstanceInfo(user.host).then((info) => {
if (!cancelled) {
setRemoteInstance(info);
}
});
} else {
setRemoteInstance(null);
setRemoteInstanceIconFailed(false);
}
return () => {
cancelled = true;
};
}, [user?.host]);2. 非Hexカラーでの背景色問題
const isHexColor = validatedThemeColor?.startsWith("#");
const badgeBgColor = isHexColor
? `${validatedThemeColor}15`
: "var(--bg-tertiary)";これにより、rgb/hsl形式のカラーでも安全にフォールバック値が使用されます。 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/frontend/src/components/user/UserProfile.tsx`:
- Around line 245-266: The current useEffect invokes
getRemoteInstanceInfo(user.host) but does not handle promise rejection, causing
unhandled Promise rejections; update the effect to await the fetch inside an
async function (or attach .catch) and handle errors: call
setRemoteInstance(null) and/or setRemoteInstanceIconFailed(true) on failure and
log the error, while still respecting the cancelled flag before setting state;
specifically modify the useEffect body that calls getRemoteInstanceInfo to wrap
the call in try/catch (or add .catch) and ensure you only call setRemoteInstance
or setRemoteInstanceIconFailed when cancelled === false.
🧹 Nitpick comments (2)
packages/frontend/src/components/user/UserProfile.tsx (2)
86-117: コメントと実装の整合性を取ると読みやすいです。
lowercaseしていないので、コメントを合わせるか実装を追加してください。♻️ 変更案
- // Trim and lowercase for consistent matching - const trimmed = color.trim(); + // Trim for consistent matching + const trimmed = color.trim();
573-620: 長い@username@host の折返しを追加すると安全です。
Federated な長いホスト名でレイアウト崩れが起きやすいので、wrap-break-wordの付与を検討してください。Based on learnings, ...♻️ 変更案
- <p className="text-gray-600 dark:text-gray-400"> + <p className="text-gray-600 dark:text-gray-400 wrap-break-word">
| // Load remote instance info for remote users | ||
| useEffect(() => { | ||
| let cancelled = false; | ||
|
|
||
| if (user?.host) { | ||
| // Reset state before fetching new instance info | ||
| setRemoteInstance(null); | ||
| setRemoteInstanceIconFailed(false); | ||
| getRemoteInstanceInfo(user.host).then((info) => { | ||
| if (!cancelled) { | ||
| setRemoteInstance(info); | ||
| } | ||
| }); | ||
| } else { | ||
| setRemoteInstance(null); | ||
| setRemoteInstanceIconFailed(false); | ||
| } | ||
|
|
||
| return () => { | ||
| cancelled = true; | ||
| }; | ||
| }, [user?.host]); |
There was a problem hiding this comment.
取得失敗時の未処理 Promise を避けてください。
getRemoteInstanceInfo の失敗を握りつぶすとコンソール警告や状態不整合の原因になります。
🐛 変更案
- getRemoteInstanceInfo(user.host).then((info) => {
- if (!cancelled) {
- setRemoteInstance(info);
- }
- });
+ getRemoteInstanceInfo(user.host)
+ .then((info) => {
+ if (!cancelled) {
+ setRemoteInstance(info);
+ }
+ })
+ .catch((err) => {
+ if (!cancelled) {
+ console.error("Failed to load remote instance info:", err);
+ setRemoteInstance(null);
+ }
+ });📝 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.
| // Load remote instance info for remote users | |
| useEffect(() => { | |
| let cancelled = false; | |
| if (user?.host) { | |
| // Reset state before fetching new instance info | |
| setRemoteInstance(null); | |
| setRemoteInstanceIconFailed(false); | |
| getRemoteInstanceInfo(user.host).then((info) => { | |
| if (!cancelled) { | |
| setRemoteInstance(info); | |
| } | |
| }); | |
| } else { | |
| setRemoteInstance(null); | |
| setRemoteInstanceIconFailed(false); | |
| } | |
| return () => { | |
| cancelled = true; | |
| }; | |
| }, [user?.host]); | |
| // Load remote instance info for remote users | |
| useEffect(() => { | |
| let cancelled = false; | |
| if (user?.host) { | |
| // Reset state before fetching new instance info | |
| setRemoteInstance(null); | |
| setRemoteInstanceIconFailed(false); | |
| getRemoteInstanceInfo(user.host) | |
| .then((info) => { | |
| if (!cancelled) { | |
| setRemoteInstance(info); | |
| } | |
| }) | |
| .catch((err) => { | |
| if (!cancelled) { | |
| console.error("Failed to load remote instance info:", err); | |
| setRemoteInstance(null); | |
| } | |
| }); | |
| } else { | |
| setRemoteInstance(null); | |
| setRemoteInstanceIconFailed(false); | |
| } | |
| return () => { | |
| cancelled = true; | |
| }; | |
| }, [user?.host]); |
🤖 Prompt for AI Agents
In `@packages/frontend/src/components/user/UserProfile.tsx` around lines 245 -
266, The current useEffect invokes getRemoteInstanceInfo(user.host) but does not
handle promise rejection, causing unhandled Promise rejections; update the
effect to await the fetch inside an async function (or attach .catch) and handle
errors: call setRemoteInstance(null) and/or setRemoteInstanceIconFailed(true) on
failure and log the error, while still respecting the cancelled flag before
setting state; specifically modify the useEffect body that calls
getRemoteInstanceInfo to wrap the call in try/catch (or add .catch) and ensure
you only call setRemoteInstance or setRemoteInstanceIconFailed when cancelled
=== false.
Added .catch() handler to prevent unhandled promise rejection when getRemoteInstanceInfo fails. The catch block also checks the cancellation flag to avoid state updates after cleanup.
|
@coderabbitai 指摘ありがとうございます。
getRemoteInstanceInfo(user.host)
.then((info) => {
if (!cancelled) {
setRemoteInstance(info);
}
})
.catch((err) => {
if (!cancelled) {
console.error("Failed to load remote instance info:", err);
}
}); |
|
Tip For best results, initiate chat on the files or code changes.
これで、ネットワークエラーやAPIの問題が発生しても、アプリケーションが安定して動作します。レビューで指摘した懸念事項がすべて解消されました!🎉 If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
Summary
This PR addresses two issues with the user profile page:
1. Remote User Display Improvements
@username@hostfor remote users instead of just@username2. Back Button Navigation Fix
window.history.back()torouter.back()(Waku's router)Screenshots
The instance badge appears below the username, similar to how it appears in NoteCard:
Test Plan
@user@hostFiles Changed
packages/frontend/src/components/user/UserProfile.tsxSummary by CodeRabbit
新機能
改善
✏️ Tip: You can customize this high-level summary in your review settings.