fix: change text - #839
Conversation
|
Warning Review limit reached
More reviews will be available in 7 minutes and 9 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughUpdates change stash wording to "Apply & remove" for button labels and toasts; several workspace components remove modal-based read-only guards and instead hide or gate editing controls or adjust editor readOnly sourcing; read-only modal content styling is also refined. ChangesStash action messaging
Read-only UI behavior changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/frontend/components/_features/[workspace]/editor/graphical/index.tsx (1)
23-36: ⚡ Quick winRemove
readOnlyfromGraphicalEditorPropstoo.
GraphicalEditornow ignores this prop, but the type still advertises it. That lets old callers keep compiling while silently losing the read-only behavior, which makes missed cleanup easy to ship.🤖 Prompt for 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. In `@src/frontend/components/_features/`[workspace]/editor/graphical/index.tsx around lines 23 - 36, GraphicalEditorProps still declares a readOnly prop even though GraphicalEditor ignores it; remove readOnly from the GraphicalEditorProps type definition and any related defaulting/usage to prevent callers from relying on a no-op prop. Locate the GraphicalEditorProps interface/type (used by the GraphicalEditor functional component) and delete the readOnly field, update any imports/usages that pass readOnly (or adjust call sites to stop supplying it), and run type-checks to ensure no residual references to readOnly remain in GraphicalEditor, SfcEditor, FbdEditor, or LadderEditor props.
🤖 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 `@src/frontend/components/_molecules/project-tree/index.tsx`:
- Around line 741-765: The inline-rename entrypoint bypasses the read-only
check: ensure the double-click / inline rename trigger in ProjectTreeLeaf is
gated by checking !isReadOnly before calling setIsEditing or invoking the rename
flow (i.e., update the onDoubleClick/inline-edit trigger to early-return if
isReadOnly), and add a defensive short-circuit at the top of handleRenameFile to
immediately return when isReadOnly is true so any direct calls cannot proceed to
executeSaveProject; refer to ProjectTreeLeaf, popoverOptions, setIsEditing, and
handleRenameFile when making these changes.
- Around line 320-340: The popoverOptions check alone doesn't prevent
double-click rename in ProjectTreeExpandableLeaf: update the
double-click/inline-edit trigger so it respects isReadOnly (e.g., change the
onDoubleClick or the call to setIsEditing to only run when isReadOnly is false),
and also add a defensive guard at the top of handleRenameFile (return early if
isReadOnly) to ensure no rename path executes in read-only mode; reference
popoverOptions, setIsEditing, ProjectTreeExpandableLeaf, and handleRenameFile
when making these changes.
---
Nitpick comments:
In `@src/frontend/components/_features/`[workspace]/editor/graphical/index.tsx:
- Around line 23-36: GraphicalEditorProps still declares a readOnly prop even
though GraphicalEditor ignores it; remove readOnly from the GraphicalEditorProps
type definition and any related defaulting/usage to prevent callers from relying
on a no-op prop. Locate the GraphicalEditorProps interface/type (used by the
GraphicalEditor functional component) and delete the readOnly field, update any
imports/usages that pass readOnly (or adjust call sites to stop supplying it),
and run type-checks to ensure no residual references to readOnly remain in
GraphicalEditor, SfcEditor, FbdEditor, or LadderEditor props.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c0d658c7-b82f-4c3d-849e-fac26e601301
📒 Files selected for processing (6)
src/frontend/components/_features/[workspace]/branches/branch-switcher-popover.tsxsrc/frontend/components/_features/[workspace]/create-element/element-card/index.tsxsrc/frontend/components/_features/[workspace]/editor/graphical/index.tsxsrc/frontend/components/_features/[workspace]/editor/monaco/index.tsxsrc/frontend/components/_molecules/project-tree/index.tsxsrc/frontend/components/_organisms/modals/read-only-project-modal.tsx
| const popoverOptions = useMemo( | ||
| () => { | ||
| const guard = (real: () => void) => () => { | ||
| if (isReadOnly) { | ||
| openModal('read-only-project') | ||
| return | ||
| } | ||
| real() | ||
| } | ||
| // Read-only ⇒ no file-level actions. Rename/Delete mutate the repo | ||
| // (rename auto-persists, delete removes the file), so they're removed | ||
| // entirely rather than funnelled to the fork modal. | ||
| if (isReadOnly) return [] | ||
| return [ | ||
| { | ||
| name: 'Rename', | ||
| onClick: guard(() => setIsEditing(true)), | ||
| onClick: () => setIsEditing(true), | ||
| icon: <PencilIcon className='h-4 w-4 stroke-brand dark:stroke-brand-light' />, | ||
| }, | ||
| { | ||
| name: 'Delete', | ||
| onClick: guard(() => handleDeleteFile()), | ||
| onClick: () => handleDeleteFile(), | ||
| icon: <CloseIcon className='h-4 w-4 stroke-brand dark:stroke-brand-light' />, | ||
| }, | ||
| ] | ||
| }, | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| [label, isReadOnly, openModal], | ||
| [label, isReadOnly], |
There was a problem hiding this comment.
Read-only rename is still reachable in ProjectTreeExpandableLeaf.
Returning [] here only removes the popover. Users can still double-click the label on Line 390 to enter inline rename mode, and handleRenameFile() still runs the rename path afterward. That leaves a write action available in the exact state this block says should have none.
Also applies to: 390-390
🤖 Prompt for 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.
In `@src/frontend/components/_molecules/project-tree/index.tsx` around lines 320 -
340, The popoverOptions check alone doesn't prevent double-click rename in
ProjectTreeExpandableLeaf: update the double-click/inline-edit trigger so it
respects isReadOnly (e.g., change the onDoubleClick or the call to setIsEditing
to only run when isReadOnly is false), and also add a defensive guard at the top
of handleRenameFile (return early if isReadOnly) to ensure no rename path
executes in read-only mode; reference popoverOptions, setIsEditing,
ProjectTreeExpandableLeaf, and handleRenameFile when making these changes.
| const popoverOptions = useMemo(() => { | ||
| // Read-only ⇒ every write action funnels into the fork modal. We | ||
| // still surface the menu items so the affordance is discoverable | ||
| // (the user can read what's there), but each click routes through | ||
| // the modal instead of the underlying handler. | ||
| const guard = (real: () => void) => () => { | ||
| if (isReadOnly) { | ||
| openModal('read-only-project') | ||
| return | ||
| } | ||
| real() | ||
| } | ||
| // Read-only ⇒ no file-level actions. Rename, Duplicate and Delete all | ||
| // mutate the repo (rename and duplicate auto-persist, delete removes the | ||
| // file), so the whole context menu is removed rather than funnelled to | ||
| // the fork modal. In-place editing of the file's contents stays allowed. | ||
| if (isReadOnly) return [] | ||
| return [ | ||
| { | ||
| name: 'Rename', | ||
| onClick: guard(() => setIsEditing(true)), | ||
| onClick: () => setIsEditing(true), | ||
| icon: <PencilIcon className='h-4 w-4 stroke-brand dark:stroke-brand-light' />, | ||
| }, | ||
| { | ||
| name: 'Duplicate', | ||
| onClick: guard(() => void handleDuplicateFile()), | ||
| onClick: () => void handleDuplicateFile(), | ||
| icon: <DuplicateIcon className='h-4 w-4 stroke-brand dark:stroke-brand-light' />, | ||
| }, | ||
| { | ||
| name: 'Delete', | ||
| onClick: guard(() => handleDeleteFile()), | ||
| onClick: () => handleDeleteFile(), | ||
| icon: <CloseIcon className='h-4 w-4 stroke-brand dark:stroke-brand-light' />, | ||
| }, | ||
| ] | ||
| }, [handleDeleteFile, handleDuplicateFile, setIsEditing, isReadOnly, openModal]) | ||
| }, [handleDeleteFile, handleDuplicateFile, setIsEditing, isReadOnly]) | ||
|
|
There was a problem hiding this comment.
Double-click still bypasses the read-only gating in ProjectTreeLeaf.
This hides the context menu, but Line 807 still enables inline rename. From there handleRenameFile() can rename the leaf and, for several leaf types, immediately call executeSaveProject(...). Please gate the inline rename entrypoint with !isReadOnly too, and ideally short-circuit the rename handler as a backstop.
Also applies to: 807-807
🤖 Prompt for 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.
In `@src/frontend/components/_molecules/project-tree/index.tsx` around lines 741 -
765, The inline-rename entrypoint bypasses the read-only check: ensure the
double-click / inline rename trigger in ProjectTreeLeaf is gated by checking
!isReadOnly before calling setIsEditing or invoking the rename flow (i.e.,
update the onDoubleClick/inline-edit trigger to early-return if isReadOnly), and
add a defensive short-circuit at the top of handleRenameFile to immediately
return when isReadOnly is true so any direct calls cannot proceed to
executeSaveProject; refer to ProjectTreeLeaf, popoverOptions, setIsEditing, and
handleRenameFile when making these changes.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/frontend/components/_features/[workspace]/source-control/commit-details.tsx (1)
19-26: ⚡ Quick winUse selector-based store subscriptions instead of destructuring full store state.
At Line 19-Line 26,
useOpenPLCStore()subscribes this component to the full store. Please switch to selector hooks for the exact slices used (project.meta.path,workspace.isReadOnly,modalActions.openModal,sharedWorkspaceActions.handleOpenProjectResponse) to follow repo rules and avoid unnecessary rerenders.As per coding guidelines: "Use Zustand auto-generated selector hooks:
useOpenPLCStore((s) => s.sliceName.property)"🤖 Prompt for 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. In `@src/frontend/components/_features/`[workspace]/source-control/commit-details.tsx around lines 19 - 26, The component currently subscribes to the entire store via useOpenPLCStore() which causes unnecessary rerenders; replace that destructuring with selector-based subscriptions: call useOpenPLCStore((s) => s.project.meta.path) for storedProjectId, useOpenPLCStore((s) => s.workspace.isReadOnly) for isReadOnly, useOpenPLCStore((s) => s.modalActions.openModal) for openModal, and useOpenPLCStore((s) => s.sharedWorkspaceActions.handleOpenProjectResponse) for the sharedWorkspaceActions handler, then update any local references to use those selected values/functions.
🤖 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.
Nitpick comments:
In
`@src/frontend/components/_features/`[workspace]/source-control/commit-details.tsx:
- Around line 19-26: The component currently subscribes to the entire store via
useOpenPLCStore() which causes unnecessary rerenders; replace that destructuring
with selector-based subscriptions: call useOpenPLCStore((s) =>
s.project.meta.path) for storedProjectId, useOpenPLCStore((s) =>
s.workspace.isReadOnly) for isReadOnly, useOpenPLCStore((s) =>
s.modalActions.openModal) for openModal, and useOpenPLCStore((s) =>
s.sharedWorkspaceActions.handleOpenProjectResponse) for the
sharedWorkspaceActions handler, then update any local references to use those
selected values/functions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 70ecfe39-122b-4e68-be93-dad7a151e109
📒 Files selected for processing (1)
src/frontend/components/_features/[workspace]/source-control/commit-details.tsx
1 participant
Summary by CodeRabbit
Bug Fixes
UI Changes
Behavior