-
Notifications
You must be signed in to change notification settings - Fork 64
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
✨ Add presence support #48
base: master
Are you sure you want to change the base?
Conversation
458d825
to
5b3fb00
Compare
lib/json0.js
Outdated
|
||
// Deletions should be treated as insertions: transforming | ||
// by them should result in a no-op (null presence), so | ||
// let's just pretend that deletions are oi to get the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "pretend that deletions are oi" comment is confusing on first read, would be worth elaborating a bit more on how that works
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍🏼 I've tried to clarify
README.md
Outdated
|
||
The format of a `json0` presence object follows a similar syntax to its ops: | ||
|
||
[{p: ['key', 123], v: 0}] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From the implementation, this shouldn't have an array wrapping it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, docs were wrong. I've updated.
lib/json0.js
Outdated
for (var i = 0; i < op.length; i++) { | ||
var component = op[i]; | ||
// Set side as 'right' because we always want the op to win ties, since | ||
// our transformed "op" isn't really an op |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be worth mentioning that for json0, this is just to handle subpath changes as a result of list move, insert, delete.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
lib/json0.js
Outdated
delete component.od; | ||
} | ||
|
||
if ('ld' in component) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm curious if this breaks the json.transform(transformed, [component], 'right')
lower down, if you, say, have presence on index 4 of an array and do a ld
on index 2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great catch. Fixed with a path check.
7d721e7
to
fe0bd74
Compare
This change adds support for the `transformPresence()` method that [`sharedb` uses][1]. We add support for both `text0` and `json0`. `text0` ------- The `text0` implementation leans on the existing [`transformPosition`][2], and takes its form and tests from [`rich-text`][3]. Its shape takes the form: ```js { index: 3, length: 5, } ``` Where: - `index` is the cursor position - `length` is the selection length (`0` for a collapsed selection) `json0` ------- The `json0` implementation has limited functionality because of the limitations of the `json0` type itself: we handle list moves `lm`, but cannot infer any information when moving objects around the tree, because the `oi` and `od` operations are destructive. However, it will attempt to transform embedded subtypes that support presence. Its shape takes the form: ```js { p: ['key', 123], v: {}, } ``` Where: - `p` is the path to the client's position within the document - `v` is the presence value The presence value `v` can take any arbitrary value (in simple cases it may even be omitted entirely). The exception to this is when using subtypes, where `v` should take the presence shape defined by the subtype. For example, when using `text0`: ```js { p: ['key'], v: {index: 5, length: 0}, } ``` [1]: share/sharedb#322 [2]: https://github.com/ottypes/json0/blob/90a3ae26364c4fa3b19b6df34dad46707a704421/lib/text0.js#L147 [3]: ottypes/rich-text#32
fe0bd74
to
58eea07
Compare
Fixes #30
This change adds support for the
transformPresence()
method thatsharedb
uses.We add support for both
text0
andjson0
.text0
The
text0
implementation leans on the existingtransformPosition
, and takes its form and tests fromrich-text
.Its shape takes the form:
Where:
index
is the cursor positionlength
is the selection length (0
for a collapsed selection)json0
The
json0
implementation has limited functionality because of thelimitations of the
json0
type itself: we handle list moveslm
, butcannot infer any information when moving objects around the tree,
because the
oi
andod
operations are destructive.However, it will attempt to transform embedded subtypes that support
presence.
Its shape takes the form:
Where:
p
is the path to the client's position within the documentv
is the presence valueThe presence value
v
can take any arbitrary value (in simple cases itmay even be omitted entirely).
The exception to this is when using subtypes, where
v
should take thepresence shape defined by the subtype. For example, when using
text0
: