Repository navigation
fix(canvas): show unavailable state instead of blank Settings panel - #2275
Conversation
PublishControls returns null when publishing is unconfigured, disabled,
or unavailable to a read-only viewer. In the content header that was
harmless (one fewer button among several), but the canvas Settings tab
renders it as the sole content — selecting the tab produced a blank
panel with no explanation.
Add a variant prop ('header' | 'panel') so the panel usage renders an
explanatory message instead of null, while the header usage keeps its
existing silent behavior.
📝 WalkthroughWalkthrough
ChangesPublish control display variants
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6ccb924c0a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!state.available) { | ||
| if (variant === 'panel') { | ||
| return ( | ||
| <p className="text-sm text-muted-foreground"> | ||
| Publishing isn't available for this page. |
There was a problem hiding this comment.
When the status request fails because of a network error or any non-OK response other than the expected permission case (for example, the route's 500 response), the effect also sets available to false. The panel therefore permanently reports that publishing is unavailable rather than indicating that its status could not be loaded or offering a retry, misleading editors during transient outages until the component is remounted. Track request failures separately from a successful { available: false } response.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in d7babae: added a separate loadError state that's only set on a genuine load failure (non-403 non-ok response, or a caught network error). The panel variant now shows a distinct "Couldn't load publishing status. Try again shortly." message in that case, and reserves "Publishing isn't available for this page." for a definitive signal (403 permission-denied, or a successful response explicitly reporting available: false). Header variant behavior is unchanged. Added test coverage for both branches in both variants.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/web/src/components/layout/middle-content/content-header/PublishControls.tsx`:
- Around line 216-227: Update the state-loading flow in PublishControls so
request failures and caught network errors are tracked separately from a
successful response reporting available === false. In the variant === 'panel'
branch of the !state.available handling, show the “Publishing isn’t available”
message only for an explicitly successful unavailable response; preserve the
existing behavior for genuine availability results and avoid presenting failures
as permanent unavailability.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a0966ae6-8a1a-452e-9956-295cd27d0a68
📒 Files selected for processing (2)
apps/web/src/components/layout/middle-content/content-header/PublishControls.tsxapps/web/src/components/layout/middle-content/page-views/canvas/CanvasPageView.tsx
…ility Both Codex and CodeRabbit review flagged the same issue: PublishControls' status effect collapsed every non-ok response and network error into the same `available: false` state as a genuine "publishing not available" signal (403 for no permission, or a successful response explicitly reporting available: false). In the panel variant that meant a transient 5xx or network blip would render the same durable-sounding "isn't available" message. Track load failures (`loadError`) separately from a definitive 403 or a successful available:false response, and show a distinct "couldn't load" message for the former in the panel variant. Header variant behavior is unchanged (stays silent either way). Also adds a test file covering both branches for both variants.
Self-review pass: - Rename loadError/setLoadError -> hasLoadError/setHasLoadError to match this file's existing is*/has* boolean naming (isLoading, isBusy, isStale). - Rewrite PublishControls.test.tsx to use the given/should/actual/expected assert() helper from stores/__tests__/riteway, matching the established convention in the sibling ShareDialog.test.tsx (same directory) rather than raw expect() assertions. No behavior change.
/simplify pass (4 parallel review agents: reuse, simplification, efficiency, altitude) on PR #2275's diff. Reuse/efficiency/altitude came back clean; simplification flagged two worthwhile fixes: - hasLoadError was a separate useState alongside the PublishState object, requiring every fetch-outcome branch to remember two setState calls that had to stay in sync by hand. Folded it into PublishState instead, so one setState call per branch keeps `available` and `hasLoadError` atomically consistent. Also drops a reset call that was already dead code (isLoading gates the render before either field is ever read). - Added an EMPTY_STATE constant to replace the same empty-state object literal that was duplicated across 3 call sites. - Consolidated the test file's three near-identical response factories (make403Response/make500Response/makeUnavailableResponse) into one parametrized makeStatusResponse(overrides), matching the precedent in BackupDiffPreview.test.tsx. No behavior change — same 16 tests pass unmodified in assertions.
1 participant
Summary
Fixes an unresolved review finding from #2250 (comment) that was left open when that PR merged.
PublishControlsreturnsnullwhen publishing is unconfigured, disabled, or unavailable to a read-only viewer (403 from the publish-status endpoint). In the content header that's harmless — one fewer button among several others — but PR Move canvas settings into tab bar (View/Code/Forms/Settings) #2250 moved canvas publish controls into a dedicated, always-selectable "Settings" tab, wherePublishControlsis the sole content. Selecting Settings in the unavailable case produced a blank panel with no explanation and no way to tell what happened.variant?: 'header' | 'panel'prop toPublishControls: the'header'variant (default) keeps the existing silentnullbehavior; the'panel'variant (used byCanvasPageView's Settings tab) renders an explanatory message instead.hasLoadErrorstate so the panel now distinguishes:available: false) →"Publishing isn't available for this page.""Couldn't load publishing status. Try again shortly."loadError→hasLoadErrorto match this file's existingis*/has*boolean naming (isLoading,isBusy,isStale), and rewrote the new test file to use thegiven/should/actual/expectedassert()helper fromstores/__tests__/riteway, matching the established convention in the siblingShareDialog.test.tsx(same directory) rather than rawexpect()assertions./simplifypass (4 parallel review agents — reuse, simplification, efficiency, altitude): reuse/efficiency/altitude came back clean (thevariantprop and local 403-classification logic match established codebase idioms — checked againstPageWebhooksDialog.tsxand others). Simplification flagged thathasLoadErrorwas a second, easy-to-desyncuseStatealongside thePublishStateobject — folded it intoPublishStateitself so every fetch-outcome branch sets both fields atomically in onesetStatecall, added anEMPTY_STATEconstant to replace a 3x-duplicated empty-state literal, and consolidated the test file's three response-factory functions into one parametrizedmakeStatusResponse(overrides), matching the precedent inBackupDiffPreview.test.tsx. No behavior change — same 16 tests pass.Test plan
bun run typecheck(web) — passesbun run lint(web) — passes (only pre-existing, unrelated warnings)bun run knip:check— no new issuesPublishControls.test.tsx(6 cases: durable-unavailable vs load-error, for both variants) — all passCANVAS_PUBLISHING_DISABLED=trueor noPUBLISH_BUCKET) → same durable message🤖 Generated with Claude Code
https://claude.ai/code/session_01FgEDGrDoiYZnXwsY5Jg53g