Skip to content

feat: remove edit as a mode #5801

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
May 26, 2025
Merged

feat: remove edit as a mode #5801

merged 13 commits into from
May 26, 2025

Conversation

Patrick-Erichsen
Copy link
Collaborator

@Patrick-Erichsen Patrick-Erichsen commented May 22, 2025

Closes CON-1937

Summary by cubic

Removed "edit" as a selectable mode and replaced it with a simpler edit state, streamlining the UI and related logic.

  • Refactors
    • Removed the "edit" mode from mode selection and related UI components.
    • Added a new isInEdit state to track when editing is active.
    • Updated input, toolbar, and session logic to use isInEdit instead of mode checks.
    • Simplified and reorganized edit-related components and thunks.

Screenshots

Edit

image

Edit outcome

image

Mode select

image

Chat/Agent apply actions

image

Testing instructions

Don't switch between your main editor and your debug editor while testing, it will mess with redux state.

Edit

  1. Type some text into Chat
  2. Highlight code and cmd + I to enter Edit
  3. Prompt, accept, confirm edit behavior and that the original editor content from Chat is restored. Do the same using keyboard shortcut
  4. Prompt, reject, confirm edit behavior and that you're still in Edit. Do the same using keyboard shortcut

Chat/Agent

Send a message that triggers an apply, confirm UI/behavior

@Patrick-Erichsen Patrick-Erichsen requested a review from a team as a code owner May 22, 2025 21:46
@Patrick-Erichsen Patrick-Erichsen requested review from sestinj and removed request for a team May 22, 2025 21:46
Copy link

netlify bot commented May 22, 2025

Deploy Preview for continuedev canceled.

Name Link
🔨 Latest commit 3d407ee
🔍 Latest deploy log https://app.netlify.com/projects/continuedev/deploys/6833d30dc79ac80008f8c020

@dosubot dosubot bot added the size:XXL This PR changes 1000+ lines, ignoring generated files. label May 22, 2025
@Patrick-Erichsen Patrick-Erichsen requested review from RomneyDa and removed request for sestinj May 23, 2025 00:14
Copy link
Collaborator Author

@Patrick-Erichsen Patrick-Erichsen left a comment

Choose a reason for hiding this comment

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

Lot of code changes here because of file renames/reorg, cleanup of linting errors (eg adding voids), etc

The meat of the logic is the following:

  • Make edit a boolean instead of a mode
  • Enhance the lump toolbar, primarily by moving apply states into it
  • When a user enters edit, clear the editor, but save the previous content and restore it when the user exits Edit

<span>Accept</span>
<span className="xs:inline-block hidden">All</span>
</div>
<span
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed this because of space constraints on the lump, and moved it into a tooltip

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was moved into LumpToolbar/ folder where we colocate all the other toolbars

// Array because of previous multi-file edit functionality
// Keeping array to not break persisted redux for now
codeToEdit: SetCodeToEditPayload[];
applyState: ApplyState;
returnToMode: MessageModes;
lastNonEditSessionWasEmpty: boolean;
previousModeEditorContent: JSONContent | undefined;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this is how we cache/return the editor content between edits

RomneyDa
RomneyDa previously approved these changes May 23, 2025
Copy link
Collaborator

@RomneyDa RomneyDa left a comment

Choose a reason for hiding this comment

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

One more thing to think about, should we persist isInEdit to redux? Maybe not

};

useEffect(() => {
document.addEventListener("keydown", handleKeyDown);
Copy link
Collaborator

Choose a reason for hiding this comment

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

assuming this wasn't changed on this PR but this might cause a bug bc the toolCallState = generated check should be in the use effect adding/removing the listener, not in the listener itself?

@github-project-automation github-project-automation bot moved this from Todo to In Progress in Issues and PRs May 23, 2025
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label May 23, 2025
sestinj
sestinj previously approved these changes May 24, 2025
@continue-code-review
Copy link

🚨 Code Review Error

GitHub API error (422): Unprocessable Entity

Please check the logs or contact the maintainers for assistance.

RomneyDa
RomneyDa previously approved these changes May 25, 2025
@continue-code-review
Copy link

🚨 Code Review Error

GitHub API error (422): Unprocessable Entity

Please check the logs or contact the maintainers for assistance.

Copy link

@continue-code-review continue-code-review bot left a comment

Choose a reason for hiding this comment

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

📝 Incremental review for latest commit: Found 1 issues in the latest commit

@@ -292,9 +294,11 @@ export function Chat() {

return (
<>
{!!showSessionTabs && mode !== "edit" && <TabBar ref={tabsRef} />}
{!!showSessionTabs && isInEdit && <TabBar ref={tabsRef} />}

Choose a reason for hiding this comment

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

The logic change appears inconsistent. The condition changed from mode !== "edit" to isInEdit, which inverts the behavior. This means TabBar with ref will now show when in edit mode instead of when not in edit mode. Please verify this is the intended behavior, as it contradicts the logic on line 300 where !isInEdit is used to show TabBar without ref.

Copy link

@continue-code-review continue-code-review bot left a comment

Choose a reason for hiding this comment

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

📝 Incremental review for latest commit: Found 1 issues in the latest commit

Copy link
Contributor

@sestinj sestinj left a comment

Choose a reason for hiding this comment

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

Kind of a small thing but I think for people with affected themes it will be pretty bothersome. Is there a foreground theme var that is intended to work with this background var?

Screenshot 2025-05-25 at 6 03 27 PM

@dosubot dosubot bot removed the lgtm This PR has been approved by a maintainer label May 26, 2025
@continue-code-review
Copy link

✅ Latest commit looks good

@Patrick-Erichsen
Copy link
Collaborator Author

image image

bg-table-oddRow doesn't feel like the most semantic var but looks good on light/dark

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label May 26, 2025
@Patrick-Erichsen Patrick-Erichsen merged commit bc4bb89 into main May 26, 2025
35 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in Issues and PRs May 26, 2025
@Patrick-Erichsen Patrick-Erichsen deleted the pe/remove-edit-mode branch May 26, 2025 02:55
@github-actions github-actions bot locked and limited conversation to collaborators May 26, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm This PR has been approved by a maintainer size:XXL This PR changes 1000+ lines, ignoring generated files.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants