Repository navigation
[3326 by Claude Opus 5.5] Goal repository identity is clipped beyond the 320px phone viewport - #3327
Conversation
![propr-dev[bot]](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
…320px the full owner and repository name are now readable, with the owner on its own line above the repository name, which wraps if needed. No tooltip or sideways scrolling is needed. Desktop still shows the compact one-line `owner/name` chip. Cause: the chip's text doesn't wrap, and that stopped the chip from shrinking even though its width was capped at the space available. So the whole chip was wider than its column and a parent element cut it off. That is why there was no ellipsis and why the page reported no horizontal overflow. Changes: - `RepositoryChip.tsx`: a new opt-in `stackOnNarrow` setting for the shared chip. Below 640px (Tailwind's `sm` breakpoint) it puts the owner above the repository name, lets both wrap, and lets the chip shrink to fit. On phones the chip's text is sized relative to the page's base font size, so it grows when the reader enlarges text. From 640px up it is the normal one-line chip. The slash is hidden on phones but screen readers still read it. Other chips (the Goal list, global search) don't use the setting and are unchanged. - `GoalsPage.tsx`: only the Goal detail header turns the setting on. - `e2e/creator-attribution.pw.ts`: an existing test that found the repository text through the chip's `truncate` style class now finds it by its test id, because the stacked chip only truncates from 640px up. Tests: - Browser tests: I added `e2e/goal-repository-identity.pw.ts`, which runs in Chromium against the built app. It uses the slug from the issue (`propr-ultrafix/propr-desktop-acceptance-20260909`) and a much longer owner/repository pair. It covers: - 320px and 390px widths. - 320px with the page's base text at 150% and 200%. - 1440px desktop. - native Linux desktop-app windows at 320px and at the zoomed 406px size. - the Goal list at 320px, which should still show one truncated line. The tests measure where every line of the chip's text actually lands and check it sits inside the header content, the viewport and every parent that could cut it off. They also check the owner is above the name and nothing is hidden behind an ellipsis. Model, elapsed time, completed status, requester, Open draft PR and the menu are still visible and on screen. - Check against the old code: on the unfixed code, the 320px case fails on the measurement itself: the chip's right edge is at 376.8px, matching the ~375px in the issue. - Results: the new spec passes 13/13. The existing Goal browser specs (regressions, queue, attention, previews, creator attribution) pass. All 3047 unit tests pass (including two new chip tests), and typecheck and lint are clean. The independent Astra review and the real native/mobile check on a device still need to happen; I tested native windows only with the simulated desktop shell in Chromium. Nothing was merged or published.
|
/review gpt-6-astra Review exact head f950f87 for the actual 320px Goal-detail repository clipping defect. Check owner/name full visibility, real text-line and clipping-ancestor bounds, root-text enlargement, accessibility slash, compact list preservation and desktop creator baseline. Document overflow alone is insufficient. Root exact-source tests and actual-data rendered phone checks remain separate; no signed-native or physical-phone claim yet. Do not merge or publish. |
|
✅ AI Code Review Complete requested by @integry Posted 1 review: |
🔍 AI Code Review — gpt-6-astraOverall EvaluationThe supplied diff for head ✅ Scoped wrapping — Only Goal detail opts into stacking. Both owner and name can wrap anywhere, and the chip and inner text can shrink. Default compact consumers retain their existing rendering. ✅ Responsive identity preservation — The slash remains in the DOM without ✅ Relevant regression coverage — Added tests inspect text-line and clipping-ancestor horizontal bounds, cover 320px and enlarged root text, and preserve compact-list checks. The desktop creator-baseline assertion remains intact with an updated selector. This was a static review; no commands or tests were run. Exact-source test execution and actual-data rendered phone inspection remain separate, unverified evidence. No signed-native or physical-phone validation is claimed. The supplied issue excerpt is truncated; the supplied diff is complete. Merge blockersNo merge blockers. SuggestionsThese are optional follow-ups and are not sent to S1: 🟢 Strengthen clipping geometry assertionsIn S2: 🟢 Verify accessible slash exposureThe DOM retains the slash, but text-content and ScoreThe implementation is focused and its proposed coverage addresses the reported failure mechanism. No verified code blocker or current check failure is present, but the requested rendered validation is unverified; six supplied checks remain pending. Score: 7/10 🤖 Review Details
Copy and edit to choose records from this review: 🤖 Review by ProPR |
|
CI failed: Full Test Suite Shard 2/4 Please investigate and fix this CI failure.
Failure evidence
|
|
✅ Applied the requested follow-up changes in commit b35208f Summary of ChangesI fixed the failure in Full Test Suite Shard 2/4 by updating one test file. Your Goal header change didn't cause it. Cause: the shard failed on a single suite, Commit f5e3510 ("wait in place behind a blocked predecessor follow-up handoff") made Fix: I added the two missing functions to the fake in Verification:
This is a test-only change with nothing visible on screen, so I made no previews. 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
|
/merge Integrate current main including merged PR #3321 (960a437). That reviewed change contains the logger containment fix, ip-address 10.7.4 lockfile resolution and the missing delayPRCommentJobInPlace/predecessorWaitDelay recovery-fixture mock exports. PR3325 CI at c5c51b3 demonstrably failed importing the first missing export from that old fixture; sanitized shard2 output retained independently. Preserve this PR's reviewed UI changes and assertions; do not create another workaround for a fixture already fixed on main. ProPR owns Git finalization. Do not merge this PR into main, publish or deploy. Root will inspect the exact resulting head, refresh Astra review and verify current checks separately. |
…g one test file. Your Goal header change didn't cause it. I fixed the failure in Full Test Suite Shard 2/4 by updating one test file. Your Goal header change didn't cause it. **Cause:** the shard failed on a single suite, `test/prPublicationRecoveryJob.test.ts`. It errored before running any tests: ``` SyntaxError: The requested module './prCommentCollisionRecovery.js' does not provide an export named 'delayPRCommentJobInPlace' ``` Commit f5e3510 ("wait in place behind a blocked predecessor follow-up handoff") made `src/jobs/prCommentHandoffIntegration.ts` import `delayPRCommentJobInPlace` and `predecessorWaitDelay` from `prCommentCollisionRecovery.js`. This suite replaces that module with a fake that lists its exports by hand, and neither function was on the list, so the module failed to load. **Fix:** I added the two missing functions to the fake in `test/prPublicationRecoveryJob.test.ts:309`, as no-op stubs. Nothing outside that test file changed. **Verification:** - Run through the project's test runner, the suite now passes 39/39. - I also checked the other suites that fake or import these modules. Six failed when I ran them with bare `node --test`, so I re-ran each failing one through the runner. All of them pass except the one I fixed. This is a test-only change with nothing visible on screen, so I made no previews. PR: #3327 Comment by: @propr-dev[bot] (ID: 6353894664) Model: Claude Opus 5.5
|
🔀 Resolved merge conflicts from Conflicting Files
Resolution SummaryI resolved the one merge conflict, in What the conflict was about:
How I resolved it: I kept Why: Both versions do the same job, so neither side's intent is lost. Checks:
The other staged changes from the merge ( 🤖 Resolution Details
System-triggered merge conflict resolution |
…ository-identity--20261022-2313-7c6 Automatically resolved merge conflicts after merging target branch into PR branch. Resolved conflicts in: - test/prPublicationRecoveryJob.test.ts PR: #3327 Model: Claude Opus 5.5
|
🔀 Auto-merged No conflicts were found — the merge was verified by an AI agent. 🤖 Verification Details
System-triggered merge conflict resolution |
|
/review gpt-6-astra Review exact main-integrated head 6b7bfed. Root full compare from previously reviewed f950f87 changes exactly four files: desktop logger, logger tests, root lockfile ip-address10.7.4, and recovery test mock. Every changed blob exactly equals reviewed/green main960a4375 (#3321); all UI code/tests and other tracked files are unchanged. Check composition and continued mobile/desktop behavior rather than assuming the earlier head review covers this head automatically. Independent root evidence at f950:65 targeted units,82 Chromium/WebKit browser cases, real completed Goal data at320/390 and24/32px root-font scaling, served bundle/source-map byte binding, actual trusted touch dragging, text-line hit testing and directly inspected stable screenshots. No signed combined-native or physical-phone acceptance is claimed. Root current source audits and current-headCI remain separate gates. Do not merge, publish or deploy. |
|
✅ AI Code Review Complete requested by @integry Posted 1 review: |
🔍 AI Code Review — gpt-6-astraOverall EvaluationThe change addresses the reported 320px clipping defect with an opt-in wrapping layout while preserving compact chips elsewhere. No merge-blocking defect is evident in the supplied diff; merge readiness is conditional on the pending current-head checks completing successfully. ✅ Scoped layout change — Only the Goal detail header enables ✅ Readable repository identity — Shrinkable containers and ✅ Relevant regression coverage — Browser assertions inspect text-line and clipping-ancestor bounds, enlarged root text, compact list behavior, and desktop creator alignment. This was a static review; no commands or tests were run. The reported integration blob equality and historical browser results were not independently verified. The supplied PR diff shows no interaction with the logger, dependency resolution, or recovery fixture changes described in the integration context. The issue excerpt is truncated, so assessment covers its visible requirements. Merge blockersNo merge blockers. SuggestionsThese are optional follow-ups and are not sent to S3: 🟢 Clarify native simulation labelsIn ScoreThe implementation is localized and backed by regression assertions targeting the actual clipping failure. No current-head failures are reported; seven checks remain pending, and runtime evidence was not independently reproduced during this review. Score: 8/10 🤖 Review Details
Copy and edit to choose records from this review: 🤖 Review by ProPR |
AI Implementation Summary
Closes #3326
Branch:
3326/claude-opus-5-5-goal-repository-identity--20261022-2313-7c6Commits: ✅ Changes committed (f950f87)
AI Processing Completed
Execution Details:
Summary:
I've fixed the clipped repository name on the Goal detail header. At 320px the full owner and repository name are now readable, with the owner on its own line above the repository name, which wraps if needed. No tooltip or sideways scrolling is needed. Desktop still shows the compact one-line
owner/namechip.Cause: the chip's text doesn't wrap, and that stopped the chip from shrinking even though its width was capped at the space available. So the whole chip was wider than its column and a parent element cut it off. That is why there was no ellipsis and why the page reported no horizontal overflow.
Changes:
RepositoryChip.tsx: a new opt-instackOnNarrowsetting for the shared chip. Below 640px (Tailwind'ssmbreakpoint) it puts the owner above the repository name, lets both wrap, and lets the chip shrink to fit. On phones the chip's text is sized relative to the page's base font size, so it grows when the reader enlarges text. From 640px up it is the normal one-line chip. The slash is hidden on phones but screen readers still read it. Other chips (the Goal list, global search) don't use the setting and are unchanged.GoalsPage.tsx: only the Goal detail header turns the setting on.e2e/creator-attribution.pw.ts: an existing test that found the repository text through the chip'struncatestyle class now finds it by its test id, because the stacked chip only truncates from 640px up.Tests:
Browser tests: I added
e2e/goal-repository-identity.pw.ts, which runs in Chromium against the built app. It uses the slug from the issue (propr-ultrafix/propr-desktop-acceptance-20260909) and a much longer owner/repository pair. It covers:The tests measure where every line of the chip's text actually lands and check it sits inside the header content, the viewport and every parent that could cut it off. They also check the owner is above the name and nothing is hidden behind an ellipsis. Model, elapsed time, completed status, requester, Open draft PR and the menu are still visible and on screen.
Check against the old code: on the unfixed code, the 320px case fails on the measurement itself: the chip's right edge is at 376.8px, matching the ~375px in the issue.
Results: the new spec passes 13/13. The existing Goal browser specs (regressions, queue, attention, previews, creator attribution) pass. All 3047 unit tests pass (including two new chip tests), and typecheck and lint are clean.
The independent Astra review and the real native/mobile check on a device still need to happen; I tested native windows only with the simulated desktop shell in Chromium. Nothing was merged or published.
Detailed Logs:
3905be8f-647d-4c34-aa6d-8b66fb1c1105Log files stored at:
/usr/src/app/logs/executions/issue-3326-2026-10-22T23-35-09-057Z-conversation.json/usr/src/app/logs/executions/issue-3326-2026-10-22T23-35-09-057Z-output.txtLatest Conversation Messages
This PR was created automatically by ProPR after processing issue #3326.
💡 Need changes?
Comment on this PR to request refinements — the AI agent monitors comments and will update the implementation based on your feedback. Keep iterating until you're satisfied!
Visual preview
Enlarged text (150%) at 320px
The chip text scales with the root font size and still wraps in full
Original unavailable: managed storage is unavailable or the upload has expired.
Long owner and repository at 320px
Very long owner and repository names wrap inside the chip instead of overflowing
Original unavailable: managed storage is unavailable or the upload has expired.
Desktop header (unchanged compact chip)
At 1440px the metadata row keeps the one-line owner/name chip
Original unavailable: managed storage is unavailable or the upload has expired.
Native Linux shell, 320px window
Desktop-app narrow window shows the full stacked identity
Original unavailable: managed storage is unavailable or the upload has expired.
Goal header at 320px (reported repository)
propr-ultrafix/propr-desktop-acceptance-20260909: owner stacked above the full repository name, inside the 320px viewport
Original unavailable: managed storage is unavailable or the upload has expired.
Goal header at 390px
Same goal at a typical phone width
Original unavailable: managed storage is unavailable or the upload has expired.