feat(frontend): add dynamic page titles for workspace and session pages#878
feat(frontend): add dynamic page titles for workspace and session pages#878
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (6)
WalkthroughAdds metadata exports and simple layout components across integrations and projects routes, and client-side effects that update document titles for project and session pages. Five files were added or modified to include metadata or render children and to set page titles after hydration. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
🧪 CI InsightsHere's what we observed from your CI run for 95eb360. 🟢 All jobs passed!But CI Insights is watching 👀 |
components/frontend/src/app/projects/[name]/sessions/[sessionName]/layout.tsx
Outdated
Show resolved
Hide resolved
Review Queue — Not Ready to Mergechristianvogt reviewed the PR and identified a substantive issue with the implementation. The session page titles are using technical session names (like 'session-17734238534') instead of user-friendly display names, making browser tabs unhelpful. This is an unresolved design flaw that defeats the purpose of the PR. To unblock: Update the session layout to use the session's display name instead of the technical session name for the page title. |
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Martin Prpič <mprpic@redhat.com>
| // Update page title with display name when available | ||
| useEffect(() => { | ||
| const title = session?.spec?.displayName || sessionName; | ||
| if (title) { | ||
| document.title = `${title} · Ambient Code Platform`; | ||
| } | ||
| }, [session?.spec?.displayName, sessionName]); |
There was a problem hiding this comment.
If the next.js way is to use metadata, why not just make the request to obtain the session name in the metadata function?
What I originally did was use the react title component which I think is a better approach vs a useEffect. While it may not make a difference here, if nested components want to add a title, on unmount it would restore the parent value.
Replaces #861