feat: show document title for Project detail pages#2091
Open
blfpd wants to merge 1 commit intogetarcaneapp:mainfrom
Open
feat: show document title for Project detail pages#2091blfpd wants to merge 1 commit intogetarcaneapp:mainfrom
blfpd wants to merge 1 commit intogetarcaneapp:mainfrom
Conversation
Contributor
Author
|
@greptileai Redo the review with current push-forced commit |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Checklist
mainbranchWhat This PR Implements
Show document.head title of Project detail pages
Fixes: #2090
Changes Made
Testing Done
./scripts/development/dev.sh startjust lint all)just test backendAI Tool Used (if applicable)
no
Additional Context
See #2090
Disclaimer Greptiles Reviews use AI, make sure to check over its work.
To better help train Greptile on our codebase, if the comment is useful and valid Like the comment, if its not helpful or invalid Dislike
To have Greptile Re-Review the changes, mention
greptileai.Greptile Summary
This PR introduces a new
+layout.svelteat the(app)/projects/level to set a dynamic<svelte:head><title>for project pages, and adds a Playwright test asserting the title on the new-project page.(app)/projects/causes it to wrap/projects(list),/projects/new, and/projects/[projectId]alike. On the list pagepage.data.projectis alwaysundefined, so the title permanently reads"Arcane | Projects | My New Project"(the placeholder), which is misleading.||vs??—page.data?.project?.name || m.compose_project_name_placeholder()treats an empty-string project name as falsy and falls back to the placeholder;??would be the correct operator here.'should have a head title') lives in theNew Compose Project Pagedescribe block, which is always on/projects/newwherepage.data.projectisundefined. The test will pass because it asserts the placeholder value, but it does not verify that the detail page (/projects/[projectId]) shows the actual project name — the stated goal of the PR.$derivedreactive declarations and{@render children()}usage are idiomatic Svelte 5 patterns.Confidence Score: 3/5
/projects(list page) will permanently display the placeholder "My New Project" in the document title. The||operator also silently swallows empty-string project names. The new test passes but only validates the fallback path, leaving the detail-page title (the PR's stated goal) without coverage. These issues lower the confidence from an otherwise straightforward change.||operator need attention.Important Files Changed
<title>frompage.data.project.namewith a placeholder fallback; because it is placed at the(app)/projects/level it applies to all three sub-routes (/projects,/projects/new,/projects/[projectId]), producing a misleading "My New Project" title on the projects list page, and uses `/projects/newrather than an actual project name on the detail page, leaving the PR's primary goal untested.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Browser navigates to /projects/*] --> B{Which route?} B -->|/projects| C[page.data = projects, counts] B -->|/projects/new| D[page.data = templates, envTemplate] B -->|/projects/projectId| E[page.data = project, editorState] C --> F{page.data?.project?.name} D --> F E --> F F -->|undefined| G[Fallback: 'My New Project'] F -->|empty string via OR| G F -->|real name| H[Use project.name] G --> I["Title: 'Arcane | Projects | My New Project'"] H --> J["Title: 'Arcane | Projects | actual-name'"] I -->|⚠️ misleading on /projects list| K[svelte:head title set] I -->|✓ correct on /projects/new| K J -->|✓ correct on detail page| KLast reviewed commit: b495010
Context used:
$stateinside$effectblo... (source)