Skip to content
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

fix: file conflict resolution #6272

Merged
merged 15 commits into from
Dec 19, 2024
Merged

fix: file conflict resolution #6272

merged 15 commits into from
Dec 19, 2024

Conversation

briangregoryholmes
Copy link
Contributor

@briangregoryholmes briangregoryholmes commented Dec 12, 2024

  • Updates conflict detection logic to be less intrusive
  • Updates alert copy
  • Adds diff/merge view
  • Adds saving/error indicators to sidebar and save button
Screenshot 2024-12-17 at 4 16 52 PM Screenshot 2024-12-17 at 10 02 23 PM Screenshot 2024-12-17 at 3 29 55 PM

@briangregoryholmes briangregoryholmes self-assigned this Dec 12, 2024
@briangregoryholmes briangregoryholmes added Type:Bug Something isn't working blocker A release blocker issue that should be resolved before a new release labels Dec 12, 2024
@nishantmonu51
Copy link
Collaborator

@briangregoryholmes : Is this ready for review ?

@briangregoryholmes briangregoryholmes marked this pull request as ready for review December 17, 2024 21:53
@briangregoryholmes briangregoryholmes marked this pull request as draft December 18, 2024 01:29
@briangregoryholmes briangregoryholmes marked this pull request as ready for review December 18, 2024 03:53
Copy link
Contributor

@ericpgreen2 ericpgreen2 left a comment

Choose a reason for hiding this comment

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

Overall, feels good!

UXQA:

  1. With autosave disabled, I add text (e.g. "abc"), then delete that same text, then attempt to save the file (the "unsaved changes" indicator was showing), it fails to save. (Jam)
  2. After I accept remote changes, I see the red exclamation point.
    image

@mindspank could we call you the Product Owner of the code editor, and can you give this a look too?

web-local/tests/sources.spec.ts Outdated Show resolved Hide resolved
web-common/src/features/entity-management/file-artifact.ts Outdated Show resolved Hide resolved
@briangregoryholmes
Copy link
Contributor Author

All fixed. I also extended the autosave debounce to 650ms, which feels better to me.

@ericpgreen2
Copy link
Contributor

The "Accept current" button is unresponsive for me: Jam

Copy link
Contributor

@ericpgreen2 ericpgreen2 left a comment

Choose a reason for hiding this comment

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

Approving so you can merge at your discretion

@briangregoryholmes briangregoryholmes merged commit 86123e3 into main Dec 19, 2024
7 checks passed
@briangregoryholmes briangregoryholmes deleted the bgh/file-conflicts branch December 19, 2024 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocker A release blocker issue that should be resolved before a new release Type:Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants