Skip to content

Conversation

@kahirokunn
Copy link
Member

@kahirokunn kahirokunn commented Dec 12, 2025

Summary

This PR improves the resource YAML editor behavior when the underlying resource changes on the server. If the user hasn’t typed anything yet, the editor auto-updates to the latest YAML. If the user has started editing, the editor won’t overwrite changes and instead shows an accessible warning with actions to reload or merge changes.

Related Issue

Fixes #XXXX

Changes

  • Updated EditButton/EditorDialog so the editor follows live server updates when not dirty.
  • Added a warning banner when the server-side resource changes during editing.
  • Added a 3-way merge option with conflict detection and safe fallbacks.

Steps to Test

  1. Open any resource details page and click Edit.
  2. Without editing, update the same resource elsewhere and confirm the editor auto-updates.
  3. Edit at least one character in the editor, then update the resource elsewhere again and confirm:
    • A warning banner appears.
    • Reload from server replaces the editor contents with the latest server version.
    • Merge my changes applies your edits onto the latest server version when there are no conflicts; otherwise you get a conflict message and no automatic overwrite happens.

Screenshots

Notes for the Reviewer

  • Merge ignores status, metadata.managedFields, and metadata.resourceVersion to reduce noisy conflicts.

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: kahirokunn
Once this PR has been reviewed and has the lgtm label, please assign sniok for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Dec 12, 2025
@kahirokunn kahirokunn marked this pull request as draft December 12, 2025 15:12
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 12, 2025
@kahirokunn kahirokunn force-pushed the editor-live-sync-merge branch from 8b191b6 to 22b5e83 Compare December 12, 2025 15:31
@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 12, 2025
@kahirokunn kahirokunn force-pushed the editor-live-sync-merge branch 2 times, most recently from 409c38b to dabe9e5 Compare December 12, 2025 15:45
@kahirokunn kahirokunn marked this pull request as ready for review December 12, 2025 15:47
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 12, 2025
@k8s-ci-robot k8s-ci-robot requested a review from sniok December 12, 2025 15:47
@kahirokunn kahirokunn force-pushed the editor-live-sync-merge branch from dabe9e5 to 6dfc8cd Compare December 12, 2025 15:48
@kahirokunn
Copy link
Member Author

@joaquimrocha The code has a bit too many lines, but I think this is the ideal editor from a user experience perspective. What do you think?
The reason is that there weren't any good 3-way merge libraries in JavaScript.

My idea is that if the server-side YAML changes, it will eventually cause an optimistic lock error. Therefore, I think it would be a better experience to reflect the server-side YAML to some extent automatically in the editor.

Signed-off-by: kahirokunn <okinakahiro@gmail.com>
@kahirokunn kahirokunn force-pushed the editor-live-sync-merge branch from 6dfc8cd to 4b11025 Compare December 14, 2025 01:37
@joaquimrocha
Copy link
Contributor

I think this is likely too complex for us to have it in time for this week's release, but let's try for the next one (and review it as soon as we can).

@joaquimrocha joaquimrocha added this to the v0.40.0 milestone Dec 15, 2025
@kahirokunn
Copy link
Member Author

I think the source code might have even more room for improvement, but I believe the PR and the video effectively convey what we're aiming to achieve.

Please let me join in on this challenge together! 💪

@sniok
Copy link
Contributor

sniok commented Dec 15, 2025

fiy there's also a somewhat related PR that we should make sure it'll work nicely together #4098

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

Labels

cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants