Skip to content

chore(desktop): default Changes view diff to inline#4497

Open
AviPeltz wants to merge 5 commits into
mainfrom
changes-view-preview-defa
Open

chore(desktop): default Changes view diff to inline#4497
AviPeltz wants to merge 5 commits into
mainfrom
changes-view-preview-defa

Conversation

@AviPeltz
Copy link
Copy Markdown
Collaborator

@AviPeltz AviPeltz commented May 13, 2026

Summary

  • Flip the persisted default for the Changes view diff mode from side-by-side to inline.
  • Bump the persist version (5 → 6) and add a migration that resets viewMode to "inline" so existing users on the old default get migrated.
  • Toolbar toggle in DiffToolbar and the split/unified mapping in LightDiffViewer are unchanged; users can still switch back.

Linear: SUPER-687

Test plan

  • Fresh install (or cleared changes-store localStorage): Changes view opens with the inline diff selected.
  • Existing install with persisted viewMode: "side-by-side": after upgrade, Changes view opens with inline selected (migration ran).
  • Toggle in DiffToolbar still flips between inline and side-by-side, and the choice persists across reload.
  • v1 file viewer (FileViewerPane) reflects the new default since it reads the same store.

Summary by cubic

Default diff is now inline in the Changes view and unified in the v2 diff pane (SUPER-687). Existing users are migrated (changes-store v6 sets viewMode: "inline", settings v1 sets diffStyle: "unified"); toggles still persist, the v1 file viewer reflects the new default, and file paths in the v2 Changes list and diff header use GitHub‑style left-ellipsis truncation to keep filenames visible with a tooltip that shows the full path.

Written for commit 9491377. Summary will update on new commits.

Summary by CodeRabbit

  • Updates

    • Default change diff view switched from side-by-side to inline; existing preferences are migrated to this new default.
    • Default diff rendering switched from split to unified; persisted settings are migrated to the new default.
  • Style

    • Improved file path and filename display across the UI for better RTL/long-path handling and updated tooltips to show full paths in monospace.

Review Change Stack

Flip the persisted default for the Changes view diff mode from
side-by-side to inline, and bump the persist version so existing
users on the old default get migrated. The toolbar toggle still lets
users switch back.

SUPER-687
@capy-ai
Copy link
Copy Markdown

capy-ai Bot commented May 13, 2026

Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 13, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e8eb60b1-b7e6-4922-9a39-2c9f5e99611e

📥 Commits

Reviewing files that changed from the base of the PR and between 777591f and 9491377.

📒 Files selected for processing (2)
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/FileRow/FileRow.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/DiffFileHeader/DiffFileHeader.tsx

📝 Walkthrough

Walkthrough

Two renderer stores change persisted diff/display defaults and add migrations (changes: viewMode -> "inline", version 6; settings: diffStyle -> "unified", version 1). FileRow and DiffFileHeader now render paths with RTL-aware truncation and show monospace path tooltips.

Changes

View Mode Default Migration

Layer / File(s) Summary
View mode default and migration
apps/desktop/src/renderer/stores/changes/store.ts
Initial state defaults to "inline" viewMode, persisted store version increments to 6, and migration logic backfills viewMode = "inline" for persisted data from versions < 6.

Settings diffStyle Default Migration

Layer / File(s) Summary
diffStyle default and migration
apps/desktop/src/renderer/stores/settings.ts
Initial diffStyle default set to "unified", persist options add version: 1 and a migrate function that normalizes older persisted states to diffStyle = "unified".

Path rendering and tooltips (FileRow & DiffFileHeader)

Layer / File(s) Summary
FileRow path display and tooltip
apps/desktop/src/renderer/routes/.../FileRow.tsx
Use basename + displayPath (basename or full file.path per hideDir), render displayed path inside a single RTL-aware truncating element with <bdi>, and change tooltip to show file.path in monospace small font.
DiffFileHeader path rendering and tooltip
apps/desktop/src/renderer/.../DiffFileHeader.tsx
Remove directory/name split; render full path in a truncating monospace span with direction: "rtl" and <bdi>, and update tooltip to display the path in small monospace text.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • superset-sh/superset#4442: Modifies FileRow.tsx rendering for the directory portion; related to the FileRow UI changes here.

Poem

🐰 I nudge defaults toward inline and unified light,
I wrap long paths so RTL reads right,
Versions step up, old states made new,
A monospace tooltip shows the full view,
I hop away, leaving tidy bytes.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: switching the default diff view mode from side-by-side to inline in the Changes view.
Description check ✅ Passed The PR description covers the key changes, migration logic, testing strategy, and includes the Linear issue reference, addressing most template sections effectively.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch changes-view-preview-defa

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 13, 2026

Greptile Summary

This PR flips the default diff view mode in the Changes view from "side-by-side" to "inline" for both fresh installs and existing users via a Zustand persist migration.

  • initialState.viewMode is changed from "side-by-side" to "inline", so new installs start on inline.
  • The persist version is bumped from 5 to 6, and a version < 6 migration unconditionally resets viewMode to "inline" — this covers both users who had the old default and users who had explicitly selected "side-by-side" in v5. The PR description explicitly documents this as intended behavior.

Confidence Score: 5/5

Safe to merge — the change is minimal, follows the existing migration pattern exactly, and covers both fresh installs and existing users.

The two-line logic change (new initialState value + one migration block) is consistent with every prior migration in the same file. Fresh installs get "inline" from initialState; existing users get it from the version 6 migration. The migration intentionally resets all v5 users, including those who had explicitly chosen "side-by-side" — the PR description calls this out explicitly in the test plan.

No files require special attention.

Important Files Changed

Filename Overview
apps/desktop/src/renderer/stores/changes/store.ts Flips initialState.viewMode to "inline", bumps persist version 5→6, and adds a v6 migration that unconditionally sets viewMode = "inline" for all previously-persisted stores.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[App starts] --> B{localStorage\nchanges-store?}
    B -- No --> C[Use initialState\nviewMode: 'inline']
    B -- Yes --> D{Persisted version?}
    D -- "< 6" --> E[Run migrate\nset viewMode = 'inline']
    D -- "= 6" --> F[Use stored viewMode\nas-is]
    E --> G[Store loaded\nviewMode: 'inline']
    C --> G
    F --> H[Store loaded\nviewMode: stored value]
Loading

Reviews (1): Last reviewed commit: "chore(desktop): default Changes view dif..." | Re-trigger Greptile

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 1 file

You’re at about 90% of the monthly review limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.

AviPeltz added 2 commits May 12, 2026 23:33
The v2 workspace reads diffStyle from the separate `settings` store,
which still defaulted to "split". Flip it to "unified" and add a
persist migration so existing users get reset.

SUPER-687
When a path is too long for the row, ellipsise the directory portion
from the left (using the existing `direction: rtl` + <bdi> pattern)
so the deepest folder and filename stay visible — matching
FileMentionNode. Basename is now shrink-0 so it never gets clipped
in favor of dir text.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/FileRow/FileRow.tsx:
- Line 125: In FileRow.tsx (the FileRow component) the span with className
"shrink-0 truncate font-medium text-foreground" prevents text-overflow ellipsis
because shrink-0 stops the element from shrinking; fix by removing "shrink-0" so
truncate can work (or alternatively add an explicit width constraint such as a
max-w/min-w class like "max-w-[200px]" or restore the previous "min-w-[120px]"
while keeping truncate) — update the span's className accordingly.
🪄 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

Run ID: 38e320c5-3598-4ba6-bd1a-58a62f85d885

📥 Commits

Reviewing files that changed from the base of the PR and between f53468c and 777591f.

📒 Files selected for processing (1)
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/FileRow/FileRow.tsx

AviPeltz added 2 commits May 12, 2026 23:49
Render the file path in the sidebar row and the diff-pane header as
a single span with direction: rtl + <bdi>, so when space is tight a
single ellipsis appears on the left and the basename stays anchored
on the right. Tooltip on hover shows the full path.

Previously the sidebar used a CSS rtl trick on a multi-span layout
that broke icon/folder alignment, and the diff header used two
separately-truncated spans that produced double ellipses
("apps/.../rou… pipelines.…").
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 2 files (changes from recent commits).

You’re at about 91% of the monthly review limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/FileRow/FileRow.tsx">

<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/FileRow/FileRow.tsx:112">
P2: The old filename label is set to `shrink-0`, which can hide the new filename/path for long rename entries. Allow this segment to shrink and truncate so the current path remains visible.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

{dir && <span className="truncate text-muted-foreground">{dir}</span>}
{oldBasename && (
<span className="truncate text-muted-foreground">
<span className="shrink-0 text-muted-foreground">
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The old filename label is set to shrink-0, which can hide the new filename/path for long rename entries. Allow this segment to shrink and truncate so the current path remains visible.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/FileRow/FileRow.tsx, line 112:

<comment>The old filename label is set to `shrink-0`, which can hide the new filename/path for long rename entries. Allow this segment to shrink and truncate so the current path remains visible.</comment>

<file context>
@@ -108,22 +108,17 @@ export const FileRow = memo(function FileRow({
-					)}
 					{oldBasename && (
-						<span className="truncate text-muted-foreground">
+						<span className="shrink-0 text-muted-foreground">
 							{oldBasename}
 							<span className="px-1">→</span>
</file context>
Suggested change
<span className="shrink-0 text-muted-foreground">
<span className="min-w-0 truncate text-muted-foreground">

Tip: Review your code locally with the cubic CLI to iterate faster.

@AviPeltz
Copy link
Copy Markdown
Collaborator Author

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant