Skip to content

Conversation

@Xiao-zhen-Liu
Copy link
Contributor

@Xiao-zhen-Liu Xiao-zhen-Liu commented Oct 27, 2022

Content

This PR adds real-time collaborative form editing to operator property editor. Only string type is affected, i.e., other field types like number or enum will remain the same. The result looks like the below demo:

Screen Recording 2022-10-27 at 11 15 09 - small

This also automatically solves the input flickering issue with previous text editors for property editor.

Implementation

  • Implemented updateYTypeFromObject which updates a YType, in particular OperatorPredicate.OperatorProperties, which in sharedModel is YType<Readonly<{ [key: string]: any }>>. Previously in updateOperatorProperty(), we used createYTypeFromObject to create a new shared object based on the updated form data. That makes collaborative form editing impossible since the form fields will never be able to connect to the same shared object. updateYTypeFromObject recursively infers which shared fields need to be updated instead of replaced, in particular Y.Text and Y.Array and Y.Map types.
  • Since the shared field needs to be "already there", empty objects as form data is not acceptable. I modified formly config to make string type have default values ("").
  • Implemented a new formly custom field wrapper, CollabWrapperComponent. Compatible with PresetWrapperComponent and will be applied to all string and texearea types. The new wrapper replaces the default input editor with quill editor, and infers the shared type that it needs to use based on its operatorID, field, and parents. Then it connects to shared text using y-quill.
  • Renamed coeditor in UserState type to user. This is because y-quill expects user type in its awareness states, to enable color for cursors.
  • Modified a few related test cases to accommodate this PR.

Limitations

  • Does not work with number fields for two reasons: A. in sharedModel there is no shared type for number since it is considered a primitive type. To enable collaborative editing would require Y.Text type. B. Even if we convert the type on-the-fly, the conversion is not elegant and the new editor does not support the increment and decrement buttons we normally see for numerical form field editors.
  • Have not implemented on breakpoint editor yet.

Compatibilities

  • Should work with Add autocomplete to filename in file scan operators #1688 since that PR uses custom field component which works on other field types, which does not conflict with form wrapper.
  • Works with PresetWrapper. BTW I also located the problems right now with preset wrapper, which is with ajv compiling (line 297 in preset.service.ts, const fitsSchema = PresetService.ajv.compile(presetSchema)(preset);) when validating the preset to apply.

@Xiao-zhen-Liu Xiao-zhen-Liu requested a review from zuozhiw October 27, 2022 18:58
Copy link
Contributor

@zuozhiw zuozhiw left a comment

Choose a reason for hiding this comment

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

looks good, three minor bugs:

  1. password field shows "***"
  2. new line when pasting into a string field
  3. bug when console has coeditor error message, model could be inconsitent with view

@Xiao-zhen-Liu
Copy link
Contributor Author

looks good, three minor bugs:

  1. password field shows "***"
  2. new line when pasting into a string field
  3. bug when console has coeditor error message, model could be inconsitent with view

Made changes for 1 and 2. I could not reproduce 3 at the moment, will fix this bug in another PR>

@Xiao-zhen-Liu Xiao-zhen-Liu merged commit 41084e6 into master Nov 10, 2022
@Xiao-zhen-Liu Xiao-zhen-Liu deleted the xiaozhen-collab-property-editor branch November 10, 2022 04:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature frontend Changes related to the frontend GUI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants