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

Feat/text editor devtool #467

Closed
wants to merge 21 commits into from
Closed

Feat/text editor devtool #467

wants to merge 21 commits into from

Conversation

easylogic
Copy link
Contributor

@easylogic easylogic commented Feb 26, 2023

What this PR does / why we need it?

When we created the text editor, I added a tool to the example to make it easier to monitor the Text CRDT type.

open http://localhost:9000/devtools/text.html

It offers the following features

  • activate, deactivate spec out
  • pause, resume spec out
  • peers list (with network status)
  • text editor
  • text structured data viewer
  • binary tree viewer
  • selected node information
  • Focus to that node when node is clicked in binary tree viewer

Any background context you want to provide?

The text CRDT type should be more visible and easily polished, and provided with a UI. It would be nice to have a type-specific monitoring tool in the future.

Scree shot

스크린샷 2023-02-27 오전 1 18 16

What are the relevant tickets?

Fixes #

Checklist

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

@codecov
Copy link

codecov bot commented Feb 26, 2023

Codecov Report

Merging #467 (57de17b) into main (e367c96) will decrease coverage by 0.48%.
The diff coverage is 26.31%.

❗ Current head 57de17b differs from pull request most recent head c71e045. Consider uploading reports for the commit c71e045 to get more accurate results

@@            Coverage Diff             @@
##             main     #467      +/-   ##
==========================================
- Coverage   89.50%   89.03%   -0.48%     
==========================================
  Files          67       67              
  Lines        5375     5271     -104     
  Branches      528      526       -2     
==========================================
- Hits         4811     4693     -118     
- Misses        381      394      +13     
- Partials      183      184       +1     
Impacted Files Coverage Δ
src/document/document.ts 71.81% <0.00%> (-7.19%) ⬇️
src/core/client.ts 74.62% <55.55%> (-2.91%) ⬇️
test/helper/helper.ts 56.52% <0.00%> (-14.32%) ⬇️
test/integration/client_test.ts 100.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@hackerwins
Copy link
Member

Thanks for the contribution I think it will be helpful to understand the data structure.
Leave a few comments.

  1. Display SplayTree rather than the LLRBTree in TreeView. We can check how the index is accessed by the user and the internal data structure is structured. The skew problem also seems to be more visible.

  2. Change activate/deactivate to attach/detach. We cannot edit the example again after the client is deactivated. It would be nice to display whether the current document was paused or resumed.

  3. Provide the function to view an existing getStructuredString dump. The dump was easy to record or deliver the status when a problem occurred.

@hackerwins hackerwins self-requested a review February 27, 2023 01:17
@hackerwins
Copy link
Member

@easylogic Thanks for your contribution.

This PR includes changes to the sync mode and logic for extracting snapshots, which can make merging a bit challenging.

I will close the issue for now and refer to it later when developing developer tools.

@hackerwins hackerwins closed this Jun 9, 2023
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