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

OT Integration - Part 2 #305

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 94 additions & 2 deletions frontend/src/packages/editor/operationManager.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
// operationManager is a centralized location for dealing with captured operations from anywhere within the editor
// these operations are shoved along and propagated to the server :)

import { BaseOperation } from "slate";
import { BaseOperation, InsertTextOperation, NodeOperation, RemoveTextOperation, TextOperation } from "slate";
import { CMSOperation } from "./api/OTClient/operation";
import { BlockData } from "./types";
import { BlockData, CMSEditorNode, CustomElement, CustomText, IsCustomElement, IsCustomTextBlock } from "./types";

export class OperationManager {
public pushToServer = (operation: CMSOperation) => {
Expand All @@ -30,4 +30,96 @@ export const slateToCmsOperation = (editorContent: BlockData, operation: BaseOpe
noop: {}
}
}
}


Varun-Sethu marked this conversation as resolved.
Show resolved Hide resolved
/**
* The semantics of Slate Operations
* - Slate models operations somewhat weirdly, theres a few key types
* - Insert/Remove text modifies the text field so the "path" isn't actually a complete path in the traditional OT sense
* - Set-node modifies a specific field
* - Paths like [a, b, c] refer to indexes in either array elements or children fields
*/

// normalizePath extends the path in a slate operation to also include the field it is editing
// for example: if we receive the slate operation {set-node underline = true of text-object} with the path [0, 0, 0]
// the normalised path will be [0, 0, 0, 4] (assuming 4 is index 4 in the text object)
const normalizePath = (editorContent: BlockData, op: NodeOperation | TextOperation): number[] => {
// quirkiness with the CMS text operation interface, the target is included in the path
if (op.type === "remove_text" || op.type === "insert_text") {
return op.path;
}

// resolve what type we're studying and fetch the field mappings for it
return [];
}

const convertTextInsertionOp = (editorContent: BlockData, op: InsertTextOperation): CMSOperation => (
{
Path: op.path,
OperationType: "insert",
IsNoOp: false,
Operation: {
$type: "stringOperation",
stringOperation: {
rangeStart: op.offset,
rangeEnd: op.offset + op.text.length,
newValue: op.text,
},
},
}
);


const convertTextRemovalOp = (editorContent: BlockData, op: RemoveTextOperation): CMSOperation => (
{
Path: op.path,
OperationType: "delete",
IsNoOp: false,
Operation: {
$type: "stringOperation",
stringOperation: {
rangeStart: op.offset - op.text.length,
rangeEnd: op.offset,
newValue: op.text,
},
},
}
);


const normalizeElementPath = (contentBlock: CMSEditorNode, op: NodeOperation | TextOperation): number[] => {
if (IsCustomElement(contentBlock)) return normalizeCustomElementPath(contentBlock as CustomElement, op);
if (IsCustomTextBlock(contentBlock)) return normalizeCustomTextPath(contentBlock as CustomText, op);

return [];
}

const normalizeCustomElementPath = (contentBlock: CustomElement, op: NodeOperation | TextOperation): number[] => {
return []
}

const normalizeCustomTextPath = (contentBlock: CustomText, op: NodeOperation | TextOperation): number[] => {
// TODO: For Mae :P - So the issue here is that fields on javascript objects are unordered, this is unlike Go where (typically) the order in which
Copy link
Contributor

@zax-xyz zax-xyz Nov 24, 2022

Choose a reason for hiding this comment

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

actual unironic comment: this isn't quite correct, objects do maintain some order:

The traversal order, as of modern ECMAScript specification, is well-defined and consistent across implementations. Within each component of the prototype chain, all non-negative integer keys (those that can be array indices) will be traversed first in ascending order by value, then other string keys in ascending chronological order of property creation.

(MDN for..in, this also extends to object methods like Object.keys() and Object.entries())

Copy link
Member Author

@Varun-Sethu Varun-Sethu Nov 24, 2022

Choose a reason for hiding this comment

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

Oh thats convenient, so should it directly correspond to the the order declared in the TS type? IE in this example:

type field = {
   a: string
   b: string
   aca: string
}

const x = { b: "hello", a: "world", aca: "there" };
const y = { a: "hello", b: "world", aca: "there" };

would x and y produce the same order for keys?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh thats convenient, so should it directly correspond to the the order declared in the TS type?

Wait ig not since it depends on order of declaration in the prototype so ig anyone can declare it in any order they want

Copy link
Contributor

@zax-xyz zax-xyz Nov 24, 2022

Choose a reason for hiding this comment

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

not necessarily as declared in the type, as those aren't available at runtime - if you do something like send an json object from the backend and parse that from the frontend, it should maintain whatever order it was sent in (assuming there's no integer keys)

Copy link
Member Author

@Varun-Sethu Varun-Sethu Nov 24, 2022

Choose a reason for hiding this comment

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

Hmm issue is though is that slate doesn't actually seem to give us a guarantee on order (for starters since it doesn't surface the entire object that its updated and only surfaces the field) because at the very least in Go I can just use reflection to map fields to their declaration order which doesn't seem possible at all in JS given each instance of a type could have a different field order

Copy link
Contributor

Choose a reason for hiding this comment

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

gg rip that's doomed then

// u lay them out is their actual order (assuming the compiler doesn't perform any struct packing optimizations, which it doesn't look like it does: https://github.com/golang/go/issues/10014)
// so this means at runtime we know the order of fields in our Go struct but not our TS struct, thus when we construct the JSON ast from the TS JSON data we are guaranteed that it conforms to the order
// in which they appear in Go structs.

// The issue however is that when we get a slate operation we need to (magically) map that to a numerical value indicating the position in the Go struct/ast, for now we will just maintain a hard coded association
// that needs to be synchronised with: https://github.com/csesoc/website/tree/main/backend/editor/OT/data/datamodels/cmsmodel but in the future we really should try and come up with a better solution
// perhaps thats ur first task as backend lead :P
// direct mapping of: https://github.com/csesoc/website/blob/main/backend/editor/OT/data/datamodels/cmsmodel/paragraph.go#L18
const fieldIndexes = {
"text": 0,
"link": 1,
"bold": 2,
"italic": 3,
"underline": 4
}

const isTextOp = op.type === "insert_text" || op.type === "remove_text";
const finalIndex = isTextOp
? fieldIndexes.text
:
Varun-Sethu marked this conversation as resolved.
Show resolved Hide resolved

}
9 changes: 8 additions & 1 deletion frontend/src/packages/editor/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ export type BlockData = Descendant[];
export type OpPropagator = (id: number, update: BlockData, operation: BaseOperation[]) => void;
export type UpdateCallback = (id: number, update: BlockData) => void;

type CustomElement = { type: "paragraph" | "heading"; children: CustomText[] };
export type CustomElement = { type: "paragraph" | "heading"; children: CustomText[] };
export type CustomText = {
textSize?: number;
text: string;
Expand All @@ -17,6 +17,13 @@ export type CustomText = {
align?: string;
};

// eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
export const IsCustomTextBlock = (o: any): o is CustomText => 'text' in o;
export const IsCustomElement = (o: any): o is CustomElement => 'type' in o && ["paragraph", "heading"].includes(o.type);

export type CMSEditorNode = CustomElement | CustomText;


export interface CMSBlockProps {
update: OpPropagator;
initialValue: BlockData;
Expand Down