You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This branch adds a diff view for comparing documents. Primarily, this is for use with versions, but it has been designed to support comparing any two documents.
There are some things that need a fast-follow, but I'd like to get the core of the feature merged into corel now.
Most of the commits are supporting work (this should probably be a stack). The main feature addition occurs in 2048903.
Navigating to diff view
Users can navigate to the diff view by setting the following URL search parameters:
| Name | Type | Required | Description |
|--------------------|-----------------------|----------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| `diffView` | `'version'` | Required | Enable *version comparison* mode UI. |
| `previousDocument` | `${string};${string}` | Optional | The document type and id identifying the *previous* document for comparison, delimited by `;` character. If this parameter isn't set, it will be inferred by finding the next existing document that immediately precedes the publication of `nextDocument`, per release layering. |
| `nextDocument` | `${string};${string}` | Required | The document type and id identifying the *next* document for comparison, delimited by `;` character. |
Example: specifying both previousDocument and nextDocument
I've added ScrollMirror to handle scroll position syncing between two panes that may have different heights. This is achieved by syncing the percentage scroll position of each pane. We could implement this directly in the Studio codebase if we wish.
Known issues
Navigating to references from the diff view is not well supported yet. The reference should be opened in a new browser window, similar to the way Cross Dataset References work.
Comments in diff view are not well supported, because the modal does not support document inspectors. I suggest we switch off comments in the diff view for now.
When opening the diff view without a previousDocument parameter, the previous document is inferred by finding the next existing document in the release stack.
Opening of the modal is deferred until this request is resolved. It would be better to open the modal immediately in a pending state.
After the request has been resolved, browser navigation is used to set the previousDocument parameter. This causes a redundant browser history entry to be created, resulting in an unusual experience when navigating back.
What to review
General implementation.
Introduction of ScrollMirror dependency.
Testing
There are no tests just yet, as I've spent more time than I expected making the code render in a test environment. First, I needed to move the diff view code that was originally organised as a plugin to be part of sanity/structure. Then I encountered many issues with existing imports in sanity/structure resolving to undefined when running in a test environment.
efps — editor "frames per second". The number of updates assumed to be possible within a second.
Derived from input latency. efps = 1000 / input_latency
Detailed information
🏠 Reference result
The performance result of sanity@latest
Benchmark
latency
p75
p90
p99
blocking time
test duration
article (title)
46ms
51ms
59ms
228ms
240ms
12.0s
article (body)
18ms
22ms
30ms
181ms
246ms
6.1s
article (string inside object)
41ms
43ms
47ms
77ms
179ms
7.2s
article (string inside array)
45ms
48ms
64ms
182ms
159ms
7.8s
recipe (name)
23ms
25ms
27ms
42ms
0ms
7.9s
recipe (description)
18ms
20ms
22ms
28ms
0ms
4.6s
recipe (instructions)
7ms
9ms
11ms
15ms
0ms
3.4s
synthetic (title)
55ms
58ms
65ms
259ms
707ms
13.7s
synthetic (string inside object)
51ms
56ms
65ms
119ms
807ms
8.5s
🧪 Experiment result
The performance result of this branch
Benchmark
latency
p75
p90
p99
blocking time
test duration
article (title)
61ms
68ms
84ms
218ms
1460ms
14.0s
article (body)
17ms
20ms
23ms
111ms
223ms
5.6s
article (string inside object)
63ms
66ms
73ms
219ms
1379ms
9.4s
article (string inside array)
66ms
67ms
77ms
348ms
1727ms
9.8s
recipe (name)
39ms
41ms
45ms
70ms
86ms
9.4s
recipe (description)
37ms
39ms
42ms
122ms
32ms
6.7s
recipe (instructions)
7ms
10ms
14ms
29ms
26ms
3.5s
synthetic (title)
152ms
161ms
183ms
606ms
7315ms
22.5s
synthetic (string inside object)
142ms
147ms
153ms
715ms
7219ms
17.8s
📚 Glossary
column definitions
benchmark — the name of the test, e.g. "article", followed by the label of the field being measured, e.g. "(title)".
latency — the time between when a key was pressed and when it was rendered. derived from a set of samples. the median (p50) is shown to show the most common latency.
p75 — the 75th percentile of the input latency in the test run. 75% of the sampled inputs in this benchmark were processed faster than this value. this provides insight into the upper range of typical performance.
p90 — the 90th percentile of the input latency in the test run. 90% of the sampled inputs were faster than this. this metric helps identify slower interactions that occurred less frequently during the benchmark.
p99 — the 99th percentile of the input latency in the test run. only 1% of sampled inputs were slower than this. this represents the worst-case scenarios encountered during the benchmark, useful for identifying potential performance outliers.
blocking time — the total time during which the main thread was blocked, preventing user input and UI updates. this metric helps identify performance bottlenecks that may cause the interface to feel unresponsive.
test duration — how long the test run took to complete.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This branch adds a diff view for comparing documents. Primarily, this is for use with versions, but it has been designed to support comparing any two documents.
There are some things that need a fast-follow, but I'd like to get the core of the feature merged into
corel
now.Most of the commits are supporting work (this should probably be a stack). The main feature addition occurs in 2048903.
Navigating to diff view
Users can navigate to the diff view by setting the following URL search parameters:
Example: specifying both
previousDocument
andnextDocument
Example: specifying only
nextDocument
(previousDocument
inferred based on release layering)New dependency: ScrollMirror
I've added ScrollMirror to handle scroll position syncing between two panes that may have different heights. This is achieved by syncing the percentage scroll position of each pane. We could implement this directly in the Studio codebase if we wish.
Known issues
previousDocument
parameter, the previous document is inferred by finding the next existing document in the release stack.previousDocument
parameter. This causes a redundant browser history entry to be created, resulting in an unusual experience when navigating back.What to review
ScrollMirror
dependency.Testing
There are no tests just yet, as I've spent more time than I expected making the code render in a test environment. First, I needed to move the diff view code that was originally organised as a plugin to be part of
sanity/structure
. Then I encountered many issues with existing imports insanity/structure
resolving toundefined
when running in a test environment.previousDocument
. Ensure the previous document is inferred as expected.I will follow up with tests as soon as these problems are resolved.