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

Add history tab and enhance visualization features to devtools #760

Merged
merged 58 commits into from
Apr 22, 2024

Conversation

chacha912
Copy link
Contributor

@chacha912 chacha912 commented Mar 4, 2024

What this PR does / why we need it?

devtools-v2.mp4

Enhancements in Devtools
Introduce new features in this PR, enhancing the functionality of Devtools:

  • Addition of History Tab
    • Introducing the History tab, where you can now view the history of document changes(TransactionEvent).
      • local-change: Highlighted in yellow , remote-change: Highlighted in blue.
      • presence-change: Identified with cursor icon , document-change: Identified with document icon.
    • Time-travel within the History tab.
      • Slide the event slider to explore the document at specific time points.
      • Click on event icon to access detailed event and operation information associated with the change.
      • Convenient navigation is also possible using the arrows on the right side of the tab.
        image
  • Enhanced Tree Graph Display
    • Mouseover tree nodes to access attributes, index, path, and pos information.
      image
  • UI Improvements
    • Resize history, document, and presence tabs by simply dragging.

Any background context you want to provide?

as-is to-be
image image

Does this PR introduce a user-facing change?:

Devtools are not available by default. If you want to use Devtools, you can do so as follows:

const doc = new yorkie.Document('docKey', {
  enableDevtools: true, // Modify the condition according to your situation
});

The event type has been modified as follows:

  • The base type has been augmented with a 'source' in addition to 'type' and 'value'.
  • The StatusChangedEvent has been added.
  • The LocalChangeEvent and RemoteChangeEvent include a new field called rawChange.
  • The event.rawChange value for local-change and remote-change events, and the event.value.snapshot for snapshot events, are set only when enableDevtools is configured as true.
  • You can subscribe to all events occurring in the document by using document.subscribe('all', (event)=>{}).
    • The events received from document.subscribe('all', (event)=>{}) are of type TransactionEvent, which is an Array<DocEvent>. TransactionEvent represents a collection of events occurring within a single transaction(e.g. doc.update()).

What are the relevant tickets?

Fixes #

Checklist

  • Added relevant tests or not required
  • Didn't break anything

Copy link

codecov bot commented Mar 4, 2024

Codecov Report

Attention: Patch coverage is 50.72993% with 135 lines in your changes are missing coverage. Please review.

Project coverage is 80.47%. Comparing base (a51f356) to head (e766f0f).

Files Patch % Lines
src/document/document.ts 51.29% 61 Missing and 14 partials ⚠️
src/devtools/index.ts 10.52% 16 Missing and 1 partial ⚠️
src/document/crdt/tree.ts 0.00% 13 Missing ⚠️
src/api/converter.ts 85.00% 6 Missing and 3 partials ⚠️
src/document/change/change.ts 0.00% 5 Missing ⚠️
src/document/json/array.ts 0.00% 2 Missing and 1 partial ⚠️
src/document/json/counter.ts 0.00% 3 Missing ⚠️
src/document/json/text.ts 0.00% 3 Missing ⚠️
src/document/json/tree.ts 0.00% 3 Missing ⚠️
src/document/operation/tree_edit_operation.ts 0.00% 3 Missing ⚠️
... and 1 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #760      +/-   ##
==========================================
- Coverage   81.92%   80.47%   -1.45%     
==========================================
  Files          59       59              
  Lines        4338     4431      +93     
  Branches      852      889      +37     
==========================================
+ Hits         3554     3566      +12     
- Misses        532      604      +72     
- Partials      252      261       +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@chacha912 chacha912 force-pushed the devtools-mvp-2-v2 branch from a86ea32 to 5b5e1ef Compare March 7, 2024 06:54
@chacha912 chacha912 marked this pull request as draft April 17, 2024 00:53
@chacha912 chacha912 force-pushed the devtools-mvp-2-v2 branch 2 times, most recently from a0da270 to 5737d82 Compare April 17, 2024 02:27
@chacha912 chacha912 marked this pull request as ready for review April 18, 2024 01:24
@chacha912 chacha912 requested a review from hackerwins April 18, 2024 01:24
Copy link
Member

@hackerwins hackerwins left a comment

Choose a reason for hiding this comment

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

Thank you for mixing DocEvent and DevtoolsEvent to harmonize. The logic has become much simpler.

I have more comments.

A. Lazy generating the operation bytes

Currently, there has been an addition of logic to directly generate the byte stream of the operation when executing changes. This logic seems to be executed even when Devtools is not enabled, potentially impacting performance.

How about optimizing the logic by lazily generating the byte stream and only building it when Devtools is enabled?

B. Move accumulating logic to Devtools

Currently, accumulating events and operations in Documents is not ideal. Would it be possible to consider managing this accumulation in Devtools instead? This could potentially help streamline the process and improve overall efficiency.

src/devtools/protocol.ts Outdated Show resolved Hide resolved
src/document/change/change.ts Outdated Show resolved Hide resolved
src/document/change/change.ts Outdated Show resolved Hide resolved
src/document/document.ts Outdated Show resolved Hide resolved
src/document/document.ts Outdated Show resolved Hide resolved
@chacha912
Copy link
Contributor Author

@hackerwins Thank you for your review. I've made the following modifications:

A. Lazy generating the operation bytes:

Modified to build byte data from events only when devtools are enabled.

B. Move accumulating logic to Devtools:

Revised to accumulate docEvents in Devtools instead of the Document.

Copy link
Member

@hackerwins hackerwins left a comment

Choose a reason for hiding this comment

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

There were a lot of complex and difficult issues in this PR. Thank you for organizing them so neatly. 🎉

@hackerwins hackerwins changed the title Add history tab to Devtools Add history tab and enhance visualization features to devtools Apr 22, 2024
@hackerwins hackerwins merged commit 4b8d5d3 into main Apr 22, 2024
2 checks passed
@hackerwins hackerwins deleted the devtools-mvp-2-v2 branch April 22, 2024 08:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants