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

feat: Support edit api v2 #1581

Merged
merged 36 commits into from
Nov 14, 2024
Merged

feat: Support edit api v2 #1581

merged 36 commits into from
Nov 14, 2024

Conversation

clepski
Copy link
Collaborator

@clepski clepski commented Oct 21, 2024

  • Support edit api v2
  • Edit events v1 will be converted event v2, see here for details
  • BREAKING CHANGE: Edit event v1 properties derived and checkValidity will be ignored
  • BREAKING CHANGE: Edit API v1 validation is no longer supported (e.g. edit api v1 checked if an elements id was unique in the document)
  • History addon now has a history state which includes editCount, canUndo and canRedo. The history addon is the owner of this state, before there were multiple components storing and changing editCount
  • Remove Editing Mixin
  • Write documentation for Edit API v2

Copy link
Contributor

github-actions bot commented Oct 23, 2024

PR Preview Action v1.4.8
Preview removed because the pull request was closed.
2024-11-14 08:36 UTC

@clepski clepski marked this pull request as ready for review October 31, 2024 15:45
@clepski clepski requested a review from trusz November 6, 2024 08:52
Copy link
Contributor

@michelguerin michelguerin left a comment

Choose a reason for hiding this comment

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

Wow, I see a lot of work to make this v2 happen ! Well done :)

Through my tests and your documentation I see that the new API doesn't check validity on each change, so it seems to be a by design decision before your implementation.
I might be wrong but I also doesn't see any validation trigger elsewhere : I can make modification and download my file without any validation.

I'm not very aware of the board decisions, but it seems to be a regression from the v1 perspective (that being said, it seems buggy in the actual instance with the v1, so it might be the reason of the removal)

packages/openscd/src/addons/Editor.ts Outdated Show resolved Hide resolved
packages/openscd/src/addons/Editor.ts Outdated Show resolved Hide resolved
packages/openscd/src/addons/Editor.ts Show resolved Hide resolved
packages/openscd/src/addons/Editor.ts Show resolved Hide resolved
docs/core-api/edit-api.md Outdated Show resolved Hide resolved
docs/core-api/edit-api.md Outdated Show resolved Hide resolved
@clepski
Copy link
Collaborator Author

clepski commented Nov 12, 2024

Wow, I see a lot of work to make this v2 happen ! Well done :)

Through my tests and your documentation I see that the new API doesn't check validity on each change, so it seems to be a by design decision before your implementation. I might be wrong but I also doesn't see any validation trigger elsewhere : I can make modification and download my file without any validation.

I'm not very aware of the board decisions, but it seems to be a regression from the v1 perspective (that being said, it seems buggy in the actual instance with the v1, so it might be the reason of the removal)

The missing validate event after edit events is a bug, thanks for catching it. I will look into fixing the functionality.

The validation before editing (like checking if ids are unique on create etc) are dropped on purpose.

@clepski clepski requested a review from michelguerin November 13, 2024 12:36
@michelguerin
Copy link
Contributor

Wow, I see a lot of work to make this v2 happen ! Well done :)
Through my tests and your documentation I see that the new API doesn't check validity on each change, so it seems to be a by design decision before your implementation. I might be wrong but I also doesn't see any validation trigger elsewhere : I can make modification and download my file without any validation.
I'm not very aware of the board decisions, but it seems to be a regression from the v1 perspective (that being said, it seems buggy in the actual instance with the v1, so it might be the reason of the removal)

The missing validate event after edit events is a bug, thanks for catching it. I will look into fixing the functionality.

The validation before editing (like checking if ids are unique on create etc) are dropped on purpose.

Wow, it was just due to the missing async keyword ? Haha... 🥲
Well done :)

@clepski clepski merged commit 14e933e into main Nov 14, 2024
7 checks passed
@clepski clepski deleted the feat/edit-api-v2 branch November 14, 2024 08:35
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.

2 participants