search

hackerwins / codepair-old

Real-time markdown editor for interviews, meetings and more...
https://codepair-old.yorkie.dev
Apache License 2.0
90 stars 29 forks source link

Add condition to delete drawing #109

Closed ppeeou closed 3 years ago

ppeeou commented 3 years ago

What this PR does / why we need it?

the error occurs when another user deletes the shape while the user is working on it.

Add conditions to delete shapes. The conditions to be added are as follows.

  1. The shape's isEditing value must be true.
  2. The user can exit while editing, so check if the user is currently connected.

Whenever there is a change in useEffect, Container is created every time, so changed it to a singleton.

Any background context you want to provide?

What are the relevant tickets?

Fixes #77

Checklist

ppeeou commented 3 years ago

I think it looks good to put the code in after this PR is merged.

hackerwins commented 3 years ago

Please resolve the conflicts.

ppeeou commented 3 years ago

@ppeeou Please explain the algorithm for whether the lock will be valid in optimistic replication.

@hackerwins Thank you for your comment :)

isEditing is valid only when a user is connected to the network

Network synchronization issues between users are merged the same before and after isEditing is added.

The reason for adding this condition is to prevent a figure from being deleted by others while a user connected to the network manipulates the figure.

The Worker stores TimeTicket in memory to manipulate the shape between user events (mousedown, mousemove ...) asynchronous actions.

getElementByID method to use the data manipulated in mousedown in mousemove If there is no data at this time(by garbage collection), a bug has occurred.

The above PR is to prevent the object from being deleted when a worker manipulates a shape with a TimeTicket.

If we do not want to use this method, if yorkie-js-sdk provides a method to validate the TimeTicket in the array proxy, it seems to be possible to solve it this way

ppeeou commented 3 years ago

This method does not solve the concurrency problem.

If an error occurs after getElementByID, try to solve it with try-catch.

I plan to add other PR