From bdeb4b36e1172d7a65f56c9a86dd2c3ab7f2b948 Mon Sep 17 00:00:00 2001 From: Aakansha Doshi Date: Sat, 13 Jul 2024 16:32:30 +0530 Subject: [PATCH 01/15] feat: rfc for supporting tables in excalidraw --- active-rfcs/0000-tables.md | 118 +++++++++++++++++++++++++++++++++++++ 1 file changed, 118 insertions(+) create mode 100644 active-rfcs/0000-tables.md diff --git a/active-rfcs/0000-tables.md b/active-rfcs/0000-tables.md new file mode 100644 index 0000000..4c29537 --- /dev/null +++ b/active-rfcs/0000-tables.md @@ -0,0 +1,118 @@ +* Start Date: 2024-07-13 +* Referenced Issues: https://github.com/excalidraw/excalidraw/issues/4847 +* Implementation PR: (leave this empty) + +# Supporting Tables in Excalidraw + +Tables is nothing but a collection of Text Containers next to each other in form of a grid with `x` rows and `y` columns. + +Lets see how the data structure of table element look like. + +A `table` element will have `rows` and `columns` defining the grid. The `table` will also have `x` , `y` coordinates for positioning on canvas and also the `width` and `height` for dimensions. + +The `table` element consists of multiple cells. Each cell will have an `id`, `x` and `y` coordinates and dimensions as well since each cell's `width/height` can be varied and some text. Each cell is a text container (rectangle with bound text). + +Below is the sample JSON representation of a `table` element + +```js +{ + "id": "table-id", + "type": "table", + "x": 100, + "y": 100, + "title": "Table Title", + "cells": [ + { + "id": "cell-1", + "tableId":"table-id", + "row": 0, + "column": 0, + "x": 100, + "y": 100, + "width": 100, + "height": 50, + "boundElements": [{id: "text-1", type: "text"}] + }, + { + "id": "cell-2", + "tableId":"table-id", + "x": 200, + "y": 100, + "row": 0, + "column": 1, + "width": 150, + "height": 50, + "boundElements": [{id: "text-2", type: "text"}] + }, + { + "id": "cell-3", + "tableId":"table-id", + "x": 100, + "y": 150, + "row": 1, + "column": 0, + "width": 120, + "height": 60, + "boundElements": [{id: "text3", type: "text"}] + }, + { + "id": "cell-4", + "tableId":"table-id", + "x": 220, + "y": 150, + "row": 1, + "column": 1, + "width": 130, + "height": 60, + "boundElements": [{id: "text-4", type: "text"}] + } + ], + "angle": 0, + "strokeColor": "#000000", + "backgroundColor": "transparent", + "fillStyle": "hachure", + "strokeWidth": 1, + "strokeStyle": "solid", + "roughness": 0, + "opacity": 100, + "groupIds": [], + "seed": 1, + "version": 1, + "versionNonce": 1, + "isDeleted": false, + "boundElements": null, + "link": null, + "locked": false +} + +``` + +A table will have a `title` as well. +Each cell has a `tableId` to identify the `table` which it is connected to when interacting with the cell eg resizing, typing text etc. + +## Adding and Deleting rows / columns + +To start with we can keep it simple and just allow to add +3 x 3 grid table. Once table is added we can have a add row and add column button to increase the rows and columns similar to how **Miro** has. + +![uploaded image](https://i.imgur.com/X21LzGR.png) + +Similarly and entire `row` and `column` can be removed by right clicking a cell. We just set the attribute `isDeleted` to true for all cells in that row / column and since we are storing the `row` and `column` index in each cell, it will be easy to identify which cells to remove. + +## Interacting with tables + +Similar to other shape tool, the table tool will be resizable, movable etc. + +Whenever user clicks on the cell - show a text box to updated or add the text. The text will start wrapping as per the cell dimensions similar to how it works for text container. + +Additionally if a user clicks on the cell stroke (width / height) should be adjustable. So we need to do a hit test whether the cell stroke is being hit and allow the user to adjust dimensions. +But since this would be internally using rectangles so this should be possible without custom code. +What we need to support is allow the width / height of all cells in that row /column when any one of them is moved. + +Since the table is not just a single element but a collection of different elements, whenever there is an operation eg moving , resizing, we need to update all the children of the table (basically every cell). And all the cells can be identified with the `tableId`. + +## Supporting row and column headers + +Ideally the first row and column should be preserved for the row and column header. But do we need a separate distinction for headers ? (Eg showing header in diff background color or some highlighter how miro does it). + +I think the users will be able to style the headers differently once we have support for wyswyg editor, so till then lets keep it simple From cac1ad445ca0e3e495f0119945c2c7bde1655f26 Mon Sep 17 00:00:00 2001 From: Aakansha Doshi Date: Sat, 13 Jul 2024 17:02:09 +0530 Subject: [PATCH 02/15] add missing details --- active-rfcs/0000-tables.md | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/active-rfcs/0000-tables.md b/active-rfcs/0000-tables.md index 4c29537..0da764c 100644 --- a/active-rfcs/0000-tables.md +++ b/active-rfcs/0000-tables.md @@ -1,10 +1,18 @@ -* Start Date: 2024-07-13 -* Referenced Issues: https://github.com/excalidraw/excalidraw/issues/4847 -* Implementation PR: (leave this empty) +- Start Date: 2024-07-13 +- Referenced Issues: https://github.com/excalidraw/excalidraw/issues/4847 +- Implementation PR: (leave this empty) -# Supporting Tables in Excalidraw +# Summary -Tables is nothing but a collection of Text Containers next to each other in form of a grid with `x` rows and `y` columns. +Once this feature is implemented the users will be able to use tables in excalidraw. + +# Motivation + +Tables are one of the most heavily requested features in Excalidraw for past few years. This will also be a very useful feature as well hence we should consider pushing it out soon. + +# Detailed Design + +Tables are nothing but a collection of Text Containers next to each other in form of a grid with `x` rows and `y` columns. Lets see how the data structure of table element look like. From 5c2ba88a3001173322fae5643401e13388e7bb0a Mon Sep 17 00:00:00 2001 From: Aakansha Doshi Date: Sat, 13 Jul 2024 17:42:24 +0530 Subject: [PATCH 03/15] update rfcs with more details --- active-rfcs/0000-tables.md | 33 +++++++++++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/active-rfcs/0000-tables.md b/active-rfcs/0000-tables.md index 0da764c..fa5b060 100644 --- a/active-rfcs/0000-tables.md +++ b/active-rfcs/0000-tables.md @@ -29,6 +29,8 @@ Below is the sample JSON representation of a `table` element "x": 100, "y": 100, "title": "Table Title", + "rows": 2, + "columns": 2, "cells": [ { "id": "cell-1", @@ -95,9 +97,18 @@ Below is the sample JSON representation of a `table` element ``` -A table will have a `title` as well. +A table will have a `title` as well and the count of `rows` and `columns` as well. Each cell has a `tableId` to identify the `table` which it is connected to when interacting with the cell eg resizing, typing text etc. +The table consists of `cells` defining the content of each cell. +Each cell has an `id`, `x`, `y` and dimensions. Since when resizing the width and height of cells can be altered hence recording the dimensions of each cell. + +Each cell is connected to its table by `tableId`. + +The `row` and `column` helps in identifying the row and column indexes of the cell. + +Each cell has a `boundElements` to keep it in sync with text containers / labeled arrows, however we can surely rename it to `textId` to keep it simple. + ## Adding and Deleting rows / columns To start with we can keep it simple and just allow to add @@ -123,4 +134,22 @@ Since the table is not just a single element but a collection of different eleme Ideally the first row and column should be preserved for the row and column header. But do we need a separate distinction for headers ? (Eg showing header in diff background color or some highlighter how miro does it). -I think the users will be able to style the headers differently once we have support for wyswyg editor, so till then lets keep it simple +I think the users will be able to style the headers differently once we have support for wyswyg editor, so till then lets keep it simple. + +## Questions + +- Should the `cells` in the above structure be actual text elements or virtual elements which are just rendered on canvas but physically they don't exist in json? This will definately simplify the data structure, however there are some unknowns listed below. + +- With above approach of virtual elements, how will impact the overall performance of interacting with tables since we heavily rely on actual elements ? +- How will this impact collaboration ? We may need a custom logic for tables? + + +# Adoption strategy + +- If we implement this proposal, how will existing Excalidraw users adopt it? + +This is a new feature so adoption should be straight forward and they will be using it as needed. + +- Is this a breaking change? If yes how are migrating the existing Excalidraw users ? + +No this is not a breaking change (unless we change some existing implementation) so hopefully no migration will be needed. \ No newline at end of file From fd5bd54c981997b576b6343e8b0efe5173b33d33 Mon Sep 17 00:00:00 2001 From: Aakansha Doshi Date: Sat, 13 Jul 2024 17:52:28 +0530 Subject: [PATCH 04/15] remove tableId as its not needed rn --- active-rfcs/0000-tables.md | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/active-rfcs/0000-tables.md b/active-rfcs/0000-tables.md index fa5b060..c661178 100644 --- a/active-rfcs/0000-tables.md +++ b/active-rfcs/0000-tables.md @@ -34,7 +34,6 @@ Below is the sample JSON representation of a `table` element "cells": [ { "id": "cell-1", - "tableId":"table-id", "row": 0, "column": 0, "x": 100, @@ -45,7 +44,6 @@ Below is the sample JSON representation of a `table` element }, { "id": "cell-2", - "tableId":"table-id", "x": 200, "y": 100, "row": 0, @@ -56,7 +54,6 @@ Below is the sample JSON representation of a `table` element }, { "id": "cell-3", - "tableId":"table-id", "x": 100, "y": 150, "row": 1, @@ -67,7 +64,6 @@ Below is the sample JSON representation of a `table` element }, { "id": "cell-4", - "tableId":"table-id", "x": 220, "y": 150, "row": 1, @@ -98,13 +94,10 @@ Below is the sample JSON representation of a `table` element ``` A table will have a `title` as well and the count of `rows` and `columns` as well. -Each cell has a `tableId` to identify the `table` which it is connected to when interacting with the cell eg resizing, typing text etc. The table consists of `cells` defining the content of each cell. Each cell has an `id`, `x`, `y` and dimensions. Since when resizing the width and height of cells can be altered hence recording the dimensions of each cell. -Each cell is connected to its table by `tableId`. - The `row` and `column` helps in identifying the row and column indexes of the cell. Each cell has a `boundElements` to keep it in sync with text containers / labeled arrows, however we can surely rename it to `textId` to keep it simple. @@ -128,7 +121,7 @@ Additionally if a user clicks on the cell stroke (width / height) should be adju But since this would be internally using rectangles so this should be possible without custom code. What we need to support is allow the width / height of all cells in that row /column when any one of them is moved. -Since the table is not just a single element but a collection of different elements, whenever there is an operation eg moving , resizing, we need to update all the children of the table (basically every cell). And all the cells can be identified with the `tableId`. +Since the table is not just a single element but a collection of different elements, whenever there is an operation eg moving , resizing, we need to update all the children of the table (basically every cell). ## Supporting row and column headers @@ -143,7 +136,6 @@ I think the users will be able to style the headers differently once we have sup - With above approach of virtual elements, how will impact the overall performance of interacting with tables since we heavily rely on actual elements ? - How will this impact collaboration ? We may need a custom logic for tables? - # Adoption strategy - If we implement this proposal, how will existing Excalidraw users adopt it? @@ -152,4 +144,4 @@ This is a new feature so adoption should be straight forward and they will be us - Is this a breaking change? If yes how are migrating the existing Excalidraw users ? -No this is not a breaking change (unless we change some existing implementation) so hopefully no migration will be needed. \ No newline at end of file +No this is not a breaking change (unless we change some existing implementation) so hopefully no migration will be needed. From 9210ee5b80529c00de7b99fd2878734c0b7b2000 Mon Sep 17 00:00:00 2001 From: Aakansha Doshi Date: Mon, 15 Jul 2024 15:01:41 +0530 Subject: [PATCH 05/15] add virtual cell elements alternative approach --- active-rfcs/0000-tables.md | 95 +++++++++++++++++++++++++++++++++++++- 1 file changed, 93 insertions(+), 2 deletions(-) diff --git a/active-rfcs/0000-tables.md b/active-rfcs/0000-tables.md index c661178..a19ac63 100644 --- a/active-rfcs/0000-tables.md +++ b/active-rfcs/0000-tables.md @@ -129,11 +129,101 @@ Ideally the first row and column should be preserved for the row and column head I think the users will be able to style the headers differently once we have support for wyswyg editor, so till then lets keep it simple. +## Alternatives + +In the above approach the text elements with `containerId` as `cell id` will exists in the `json`. This means there will be 4 separate `text` elements in the `json`. + +Here is an alternative version - Having `cells` as virtual text elements instead of actual text elements. + +```js +{ + "id": "table-id", + "type": "table", + "x": 100, + "y": 100, + "title": "Table Title", + "rows": 2, + "columns": 2, + "cells": [ + { + "id": "cell-1", + "row": 0, + "column": 0, + "x": 100, + "y": 100, + "width": 100, + "height": 50, + "text":"Cell 1", + "fontSize": 16; + "fontFamily": 1; + + }, + { + "id": "cell-2", + "x": 200, + "y": 100, + "row": 0, + "column": 1, + "width": 150, + "height": 50, + "text":"Cell 2", + "fontSize": 16; + "fontFamily": 1; + }, + { + "id": "cell-3", + "x": 100, + "y": 150, + "row": 1, + "column": 0, + "width": 120, + "height": 60, + "text":"Cell 3", + "fontSize": 16; + "fontFamily": 1; + }, + { + "id": "cell-4", + "x": 220, + "y": 150, + "row": 1, + "column": 1, + "width": 130, + "height": 60, + "text":"Cell 4", + "fontSize": 16; + "fontFamily": 1; + } + ], + "angle": 0, + "strokeColor": "#000000", + "backgroundColor": "transparent", + "fillStyle": "hachure", + "strokeWidth": 1, + "strokeStyle": "solid", + "roughness": 0, + "opacity": 100, + "groupIds": [], + "seed": 1, + "version": 1, + "versionNonce": 1, + "isDeleted": false, + "boundElements": null, + "link": null, + "locked": false +} +``` +As you can see above the `cells` will contain all the attributes (only some of text attributes are shown above) and at the time of rendering these text elements will be drawn, however we won't be storing the text elements as separate elements. +This would simplify the data structure and reduce the hierarchy, thus helping in better maintainance + ## Questions -- Should the `cells` in the above structure be actual text elements or virtual elements which are just rendered on canvas but physically they don't exist in json? This will definately simplify the data structure, however there are some unknowns listed below. +## For Virtual Cell Elements -- With above approach of virtual elements, how will impact the overall performance of interacting with tables since we heavily rely on actual elements ? +When `cells` are virtual elements which are just rendered on canvas but physically they don't exist in json? This will definately simplify the data structure, however there are some unknowns listed below. + +- With above approach of virtual elements, how will impact the overall performance of interacting with tables since we heavily rely on actual elements ? +- Need to verify if this approach is feasable with current state of TextWYSIWYG - How will this impact collaboration ? We may need a custom logic for tables? # Adoption strategy @@ -145,3 +235,4 @@ This is a new feature so adoption should be straight forward and they will be us - Is this a breaking change? If yes how are migrating the existing Excalidraw users ? No this is not a breaking change (unless we change some existing implementation) so hopefully no migration will be needed. +``` From 55182b4a915b7254555493e0423a1372cd626579 Mon Sep 17 00:00:00 2001 From: Aakansha Doshi Date: Mon, 15 Jul 2024 15:15:15 +0530 Subject: [PATCH 06/15] tweak RFC --- active-rfcs/0000-tables.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/active-rfcs/0000-tables.md b/active-rfcs/0000-tables.md index a19ac63..6980d29 100644 --- a/active-rfcs/0000-tables.md +++ b/active-rfcs/0000-tables.md @@ -4,7 +4,7 @@ # Summary -Once this feature is implemented the users will be able to use tables in excalidraw. +Once this feature is implemented the users will be able to use tables in excalidraw. The `table` will be added to the shapes toolbar similar to other tools. # Motivation @@ -125,7 +125,7 @@ Since the table is not just a single element but a collection of different eleme ## Supporting row and column headers -Ideally the first row and column should be preserved for the row and column header. But do we need a separate distinction for headers ? (Eg showing header in diff background color or some highlighter how miro does it). +Ideally the `first` `row` and `column` should be preserved for the `row` and `column` `header`. But do we need a separate distinction for headers ? (Eg showing header in diff background color or some highlighter how miro does it). I think the users will be able to style the headers differently once we have support for wyswyg editor, so till then lets keep it simple. @@ -220,7 +220,7 @@ This would simplify the data structure and reduce the hierarchy, thus helping in ## For Virtual Cell Elements -When `cells` are virtual elements which are just rendered on canvas but physically they don't exist in json? This will definately simplify the data structure, however there are some unknowns listed below. +When `cells` are virtual elements which are just rendered on canvas but physically they don't exist as separate text elements? This will definately simplify the data structure, however there are some unknowns listed below which needs to be evaluated before we move with this approach. - With above approach of virtual elements, how will impact the overall performance of interacting with tables since we heavily rely on actual elements ? - Need to verify if this approach is feasable with current state of TextWYSIWYG From cccd1665f6ab09de038f67cbe3d5e90647dcb9e6 Mon Sep 17 00:00:00 2001 From: Aakansha Doshi Date: Wed, 17 Jul 2024 15:20:31 +0530 Subject: [PATCH 07/15] add alternate approach to ease reordering of rows/columns --- active-rfcs/0000-tables.md | 106 +++++++++++++++++++++++++++++++++++-- 1 file changed, 102 insertions(+), 4 deletions(-) diff --git a/active-rfcs/0000-tables.md b/active-rfcs/0000-tables.md index 6980d29..513c5ed 100644 --- a/active-rfcs/0000-tables.md +++ b/active-rfcs/0000-tables.md @@ -129,8 +129,15 @@ Ideally the `first` `row` and `column` should be preserved for the `row` and `co I think the users will be able to style the headers differently once we have support for wyswyg editor, so till then lets keep it simple. +## Reordering rows and columns + +This is a powerful feature and good to have. Users can drag the rows and columns and reorder them on the UI. +However I think in the first release we might not want to support this and add this as an enhancement in future releases. + ## Alternatives +### Virtual Text Elements + In the above approach the text elements with `containerId` as `cell id` will exists in the `json`. This means there will be 4 separate `text` elements in the `json`. Here is an alternative version - Having `cells` as virtual text elements instead of actual text elements. @@ -213,19 +220,107 @@ Here is an alternative version - Having `cells` as virtual text elements instead "locked": false } ``` + As you can see above the `cells` will contain all the attributes (only some of text attributes are shown above) and at the time of rendering these text elements will be drawn, however we won't be storing the text elements as separate elements. This would simplify the data structure and reduce the hierarchy, thus helping in better maintainance -## Questions - -## For Virtual Cell Elements +#### Questions When `cells` are virtual elements which are just rendered on canvas but physically they don't exist as separate text elements? This will definately simplify the data structure, however there are some unknowns listed below which needs to be evaluated before we move with this approach. -- With above approach of virtual elements, how will impact the overall performance of interacting with tables since we heavily rely on actual elements ? +- With above approach of virtual elements, how will impact the overall performance of interacting with tables since we heavily rely on actual elements ? - Need to verify if this approach is feasable with current state of TextWYSIWYG - How will this impact collaboration ? We may need a custom logic for tables? + +### Storing columns with respect to rows to ease reordering of rows and columns + +In the suggested approach, reordering might get difficult as we will have to keep track of which cells should be swapped - lets say if we swap row 1 with row 3, we will have to iterate through all the cells and check which cells have `row` attribute set to `row 1` and swap them with cells with `row` attribute set to `row 3`. + +Here is an alternate approach to ease the above process + +```js +{ + "id": "table-id", + "type": "table", + "x": 100, + "y": 100, + "title": "Table Title", + "rows": 2, + "columns": 2, + "rows": [{ + "id": "row-1", + "columns": [{ + "id": "cell-1", + "x": 100, + "y": 100, + "width": 100, + "height": 50, + "boundElements": [{id: "text-1", type: "text"}] + }, + { + "id": "cell-2", + "x": 200, + "y": 100, + "row": 0, + "column": 1, + "width": 150, + "height": 50, + "boundElements": [{id: "text-2", type: "text"}] + } + ]}, + { + "id": "row-2", + "columns:[{ + "id": "cell-3", + "x": 100, + "y": 150, + "width": 120, + "height": 60, + "boundElements": [{id: "text3", type: "text"}] + }, + { + "id": "cell-4", + "x": 220, + "y": 150, + "width": 130, + "height": 60, + "boundElements": [{id: "text-4", type: "text"}] + } + }], + "angle": 0, + "strokeColor": "#000000", + "backgroundColor": "transparent", + "fillStyle": "hachure", + "strokeWidth": 1, + "strokeStyle": "solid", + "roughness": 0, + "opacity": 100, + "groupIds": [], + "seed": 1, + "version": 1, + "versionNonce": 1, + "isDeleted": false, + "boundElements": null, + "link": null, + "locked": false +} + +``` + +As you can see now all the data related to rows stays together and whenever the rows are swapped, we can swapped the entire row between `source` and `destination` index. + +Whenever the columns are swapped, we will need to swap the cells of the `source` index in each `row` with the `destination` index in each `row`. + +This way the swapping becomes easier since we need not filter the cells based on row / column positions. + +#### Questions + +- This approach definately will ease the swapping and handling of rows and columns. Will the collaboration be impacted ? We will need to write an algorithm to handle the reconcilation for tables ? + +- Do we need `rows` and `columns` attribute separately as well ? Since these can be calculated by `rows.length` and `columns.length`. + + # Adoption strategy - If we implement this proposal, how will existing Excalidraw users adopt it? @@ -235,4 +330,7 @@ This is a new feature so adoption should be straight forward and they will be us - Is this a breaking change? If yes how are migrating the existing Excalidraw users ? No this is not a breaking change (unless we change some existing implementation) so hopefully no migration will be needed. + +``` + ``` From 3ba9c822980d3ea56aba4dd946286f09436397d9 Mon Sep 17 00:00:00 2001 From: Aakansha Doshi Date: Wed, 17 Jul 2024 15:57:33 +0530 Subject: [PATCH 08/15] add order ad timestamp to resolve conflicts --- active-rfcs/0000-tables.md | 116 ++++++++++++++++++++++++++++++++++--- 1 file changed, 107 insertions(+), 9 deletions(-) diff --git a/active-rfcs/0000-tables.md b/active-rfcs/0000-tables.md index 513c5ed..f5dc30b 100644 --- a/active-rfcs/0000-tables.md +++ b/active-rfcs/0000-tables.md @@ -232,12 +232,11 @@ When `cells` are virtual elements which are just rendered on canvas but physical - Need to verify if this approach is feasable with current state of TextWYSIWYG - How will this impact collaboration ? We may need a custom logic for tables? - -### Storing columns with respect to rows to ease reordering of rows and columns +### Storing cells with respect to rows to ease reordering of rows and columns In the suggested approach, reordering might get difficult as we will have to keep track of which cells should be swapped - lets say if we swap row 1 with row 3, we will have to iterate through all the cells and check which cells have `row` attribute set to `row 1` and swap them with cells with `row` attribute set to `row 3`. -Here is an alternate approach to ease the above process +Here is an alternate approach to ease the above process. ```js { @@ -246,11 +245,9 @@ Here is an alternate approach to ease the above process "x": 100, "y": 100, "title": "Table Title", - "rows": 2, - "columns": 2, "rows": [{ "id": "row-1", - "columns": [{ + "cells": [{ "id": "cell-1", "x": 100, "y": 100, @@ -271,7 +268,7 @@ Here is an alternate approach to ease the above process ]}, { "id": "row-2", - "columns:[{ + "cells:[{ "id": "cell-3", "x": 100, "y": 150, @@ -288,6 +285,14 @@ Here is an alternate approach to ease the above process "boundElements": [{id: "text-4", type: "text"}] } }], + "columns:": [{ + "id": "column-1", + "title": "Column 1", + }, + { + "id": "column-2", + "title": "Column 2", + }], "angle": 0, "strokeColor": "#000000", "backgroundColor": "transparent", @@ -316,9 +321,102 @@ This way the swapping becomes easier since we need not filter the cells based on #### Questions -- This approach definately will ease the swapping and handling of rows and columns. Will the collaboration be impacted ? We will need to write an algorithm to handle the reconcilation for tables ? +- This approach definately will ease the swapping and handling of rows and columns. How do we track changes when columns or rows are reordered ? + +We can add an `order` attribute which helps in tracking the changes of the cells and each cell can have a `timestamp` attribute as well to determine the latest changes. + +```js +{ + "id": "table-id", + "type": "table", + "x": 100, + "y": 100, + "title": "Table Title", + "rows": [{ + "id": "row-1", + "cells": [{ + "id": "cell-1", + "order": 1, + "x": 100, + "y": 100, + "width": 100, + "height": 50, + "boundElements": [{id: "text-1", type: "text"}], + "timestamp": 1633257616000 + }, + { + "id": "cell-2", + "order": 2, + "x": 200, + "y": 100, + "row": 0, + "column": 1, + "width": 150, + "height": 50, + "boundElements": [{id: "text-2", type: "text"}], + "timestamp": 1633257617000 + } + ]}, + { + "id": "row-2", + "cells:[{ + "id": "cell-3", + "order": 1, + "x": 100, + "y": 150, + "width": 120, + "height": 60, + "boundElements": [{id: "text3", type: "text"}], + "timestamp": 1633257618000 + + }, + { + "id": "cell-4", + "order": 2, + "x": 220, + "y": 150, + "width": 130, + "height": 60, + "boundElements": [{id: "text-4", type: "text"}], + "timestamp": 1633257619000 + + } + }], + "columns:": [{ + "id": "column-1", + "order": 1, + "title": "Column 1", + "timestamp": 1633257621000 + }, + { + "id": "column-2", + "order": 2, + "title": "Column 2", + "timestamp": 1633257622000 + }], + "angle": 0, + "strokeColor": "#000000", + "backgroundColor": "transparent", + "fillStyle": "hachure", + "strokeWidth": 1, + "strokeStyle": "solid", + "roughness": 0, + "opacity": 100, + "groupIds": [], + "seed": 1, + "version": 1, + "versionNonce": 1, + "isDeleted": false, + "boundElements": null, + "link": null, + "locked": false +} +``` +We will need to write a custom reconcilation to reconcile correctly for this case as we are adding extra attributes hence a custom approach. + +When there are updates we will compare the order and if order is updated, it means rows and columns were swapped and then compare the timestamp as well to determine the latest changes. -- Do we need `rows` and `columns` attribute separately as well ? Since these can be calculated by `rows.length` and `columns.length`. +- Will this work for multiplayer undo/redo ? # Adoption strategy From 5b4f16ebc6096461a52084fba634b02feb977675 Mon Sep 17 00:00:00 2001 From: Aakansha Doshi Date: Wed, 17 Jul 2024 18:13:02 +0530 Subject: [PATCH 09/15] add lastUpdated --- active-rfcs/0000-tables.md | 116 ++++++++++++++++++++++++++++++++----- 1 file changed, 101 insertions(+), 15 deletions(-) diff --git a/active-rfcs/0000-tables.md b/active-rfcs/0000-tables.md index f5dc30b..03d424a 100644 --- a/active-rfcs/0000-tables.md +++ b/active-rfcs/0000-tables.md @@ -232,6 +232,8 @@ When `cells` are virtual elements which are just rendered on canvas but physical - Need to verify if this approach is feasable with current state of TextWYSIWYG - How will this impact collaboration ? We may need a custom logic for tables? +Yes we will need write custom logic to handle reconcilation in `tables`, mainly attaching `timestamp` to each cell and writing a custom algorithm to resolve the conflicts and ensuring latest changes are applied to all the collaborators. + ### Storing cells with respect to rows to ease reordering of rows and columns In the suggested approach, reordering might get difficult as we will have to keep track of which cells should be swapped - lets say if we swap row 1 with row 3, we will have to iterate through all the cells and check which cells have `row` attribute set to `row 1` and swap them with cells with `row` attribute set to `row 3`. @@ -323,7 +325,7 @@ This way the swapping becomes easier since we need not filter the cells based on - This approach definately will ease the swapping and handling of rows and columns. How do we track changes when columns or rows are reordered ? -We can add an `order` attribute which helps in tracking the changes of the cells and each cell can have a `timestamp` attribute as well to determine the latest changes. +We can add an `order` attribute which helps in maintaining the correct sequence of `rows` and `columns` and `lastUpdated` helps in resolving conflicts in reordering. ```js { @@ -334,19 +336,18 @@ We can add an `order` attribute which helps in tracking the changes of the cells "title": "Table Title", "rows": [{ "id": "row-1", + "order": 1, + "lastUpdated": 1633257617000 "cells": [{ "id": "cell-1", - "order": 1, "x": 100, "y": 100, "width": 100, "height": 50, "boundElements": [{id: "text-1", type: "text"}], - "timestamp": 1633257616000 }, { "id": "cell-2", - "order": 2, "x": 200, "y": 100, "row": 0, @@ -354,31 +355,27 @@ We can add an `order` attribute which helps in tracking the changes of the cells "width": 150, "height": 50, "boundElements": [{id: "text-2", type: "text"}], - "timestamp": 1633257617000 } ]}, { "id": "row-2", + "order": 2, + "lastUpdated": 1633257617000 "cells:[{ "id": "cell-3", - "order": 1, "x": 100, "y": 150, "width": 120, "height": 60, "boundElements": [{id: "text3", type: "text"}], - "timestamp": 1633257618000 - }, { "id": "cell-4", - "order": 2, "x": 220, "y": 150, "width": 130, "height": 60, "boundElements": [{id: "text-4", type: "text"}], - "timestamp": 1633257619000 } }], @@ -386,13 +383,11 @@ We can add an `order` attribute which helps in tracking the changes of the cells "id": "column-1", "order": 1, "title": "Column 1", - "timestamp": 1633257621000 }, { "id": "column-2", "order": 2, "title": "Column 2", - "timestamp": 1633257622000 }], "angle": 0, "strokeColor": "#000000", @@ -412,12 +407,101 @@ We can add an `order` attribute which helps in tracking the changes of the cells "locked": false } ``` -We will need to write a custom reconcilation to reconcile correctly for this case as we are adding extra attributes hence a custom approach. -When there are updates we will compare the order and if order is updated, it means rows and columns were swapped and then compare the timestamp as well to determine the latest changes. +We will need to write a custom reconcilation to reconcile correctly for this case. -- Will this work for multiplayer undo/redo ? +When there are updates we will compare the order and if order is updated, it means rows and columns were reordered and compare the `lastUpdated` as well to ensure most recent update is applied to all the collaborators. + +Additionally each cell will also have a `timestamp` attribute if we consider virtual text elements (as mentioned in previous approach) to resolve conflicts when content updated on same cell. + +```js +{ + "id": "table-id", + "type": "table", + "x": 100, + "y": 100, + "title": "Table Title", + "rows": [{ + "id": "row-1", + "order": 1, + "lastUpdated": 1633257617000 + "cells": [{ + "id": "cell-1", + "x": 100, + "y": 100, + "width": 100, + "height": 50, + "content": "Cell-11", + "timestamp": 1633257618000 + }, + { + "id": "cell-2", + "x": 200, + "y": 100, + "row": 0, + "column": 1, + "width": 150, + "height": 50, + "content": "Cell-12", + "timestamp": 1633257619000 + } + ]}, + { + "id": "row-2", + "order": 2, + "lastUpdated": 1633257620000 + "cells:[{ + "id": "cell-3", + "x": 100, + "y": 150, + "width": 120, + "height": 60, + "content": "Cell-21", + "timestamp": 1633257621000 + }, + { + "id": "cell-4", + "x": 220, + "y": 150, + "width": 130, + "height": 60, + "content": "Cell-22", + "timestamp": 1633257617000 + } + }], + "columns:": [{ + "id": "column-1", + "order": 1, + "title": "Column 1", + "timestamp": 1633257617000 + }, + { + "id": "column-2", + "order": 2, + "title": "Column 2", + "timestamp": 1633257617000 + }], + "angle": 0, + "strokeColor": "#000000", + "backgroundColor": "transparent", + "fillStyle": "hachure", + "strokeWidth": 1, + "strokeStyle": "solid", + "roughness": 0, + "opacity": 100, + "groupIds": [], + "seed": 1, + "version": 1, + "versionNonce": 1, + "isDeleted": false, + "boundElements": null, + "link": null, + "locked": false +} +``` + +- Will this work for multiplayer undo/redo ? # Adoption strategy @@ -432,3 +516,5 @@ No this is not a breaking change (unless we change some existing implementation) ``` ``` + +``` From 38deb780cb25d03b82b0d7c20a771482249aa1d3 Mon Sep 17 00:00:00 2001 From: Aakansha Doshi Date: Wed, 17 Jul 2024 18:18:08 +0530 Subject: [PATCH 10/15] rename order to index --- active-rfcs/0000-tables.md | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/active-rfcs/0000-tables.md b/active-rfcs/0000-tables.md index 03d424a..5c7960d 100644 --- a/active-rfcs/0000-tables.md +++ b/active-rfcs/0000-tables.md @@ -325,7 +325,7 @@ This way the swapping becomes easier since we need not filter the cells based on - This approach definately will ease the swapping and handling of rows and columns. How do we track changes when columns or rows are reordered ? -We can add an `order` attribute which helps in maintaining the correct sequence of `rows` and `columns` and `lastUpdated` helps in resolving conflicts in reordering. +We can add an `index` attribute which helps in maintaining the correct sequence of `rows` and `columns` and `lastUpdated` helps in resolving conflicts in reordering. ```js { @@ -336,7 +336,7 @@ We can add an `order` attribute which helps in maintaining the correct sequence "title": "Table Title", "rows": [{ "id": "row-1", - "order": 1, + "index": 0, "lastUpdated": 1633257617000 "cells": [{ "id": "cell-1", @@ -359,7 +359,7 @@ We can add an `order` attribute which helps in maintaining the correct sequence ]}, { "id": "row-2", - "order": 2, + "index": 1, "lastUpdated": 1633257617000 "cells:[{ "id": "cell-3", @@ -381,12 +381,12 @@ We can add an `order` attribute which helps in maintaining the correct sequence }], "columns:": [{ "id": "column-1", - "order": 1, + "index": 0, "title": "Column 1", }, { "id": "column-2", - "order": 2, + "index": 1, "title": "Column 2", }], "angle": 0, @@ -410,7 +410,7 @@ We can add an `order` attribute which helps in maintaining the correct sequence We will need to write a custom reconcilation to reconcile correctly for this case. -When there are updates we will compare the order and if order is updated, it means rows and columns were reordered and compare the `lastUpdated` as well to ensure most recent update is applied to all the collaborators. +When there are updates we will compare the `index` and if `index` is updated, it means rows and columns were reordered and compare the `lastUpdated` as well to ensure most recent update is applied to all the collaborators. Additionally each cell will also have a `timestamp` attribute if we consider virtual text elements (as mentioned in previous approach) to resolve conflicts when content updated on same cell. @@ -423,7 +423,7 @@ Additionally each cell will also have a `timestamp` attribute if we consider vir "title": "Table Title", "rows": [{ "id": "row-1", - "order": 1, + "index": 0, "lastUpdated": 1633257617000 "cells": [{ "id": "cell-1", @@ -448,7 +448,7 @@ Additionally each cell will also have a `timestamp` attribute if we consider vir ]}, { "id": "row-2", - "order": 2, + "index": 1, "lastUpdated": 1633257620000 "cells:[{ "id": "cell-3", @@ -472,13 +472,13 @@ Additionally each cell will also have a `timestamp` attribute if we consider vir }], "columns:": [{ "id": "column-1", - "order": 1, + "index": 0, "title": "Column 1", "timestamp": 1633257617000 }, { "id": "column-2", - "order": 2, + "index": 1, "title": "Column 2", "timestamp": 1633257617000 }], @@ -518,3 +518,5 @@ No this is not a breaking change (unless we change some existing implementation) ``` ``` + +``` From a5cf5be9be5922e193aefe672c869aa0123917e8 Mon Sep 17 00:00:00 2001 From: Aakansha Doshi Date: Wed, 17 Jul 2024 18:20:10 +0530 Subject: [PATCH 11/15] remove trailing quotes --- active-rfcs/0000-tables.md | 8 -------- 1 file changed, 8 deletions(-) diff --git a/active-rfcs/0000-tables.md b/active-rfcs/0000-tables.md index 5c7960d..339d148 100644 --- a/active-rfcs/0000-tables.md +++ b/active-rfcs/0000-tables.md @@ -512,11 +512,3 @@ This is a new feature so adoption should be straight forward and they will be us - Is this a breaking change? If yes how are migrating the existing Excalidraw users ? No this is not a breaking change (unless we change some existing implementation) so hopefully no migration will be needed. - -``` - -``` - -``` - -``` From 8b02bee137796373669487d0d6108e9fc40fdf4b Mon Sep 17 00:00:00 2001 From: Aakansha Doshi Date: Thu, 1 Aug 2024 17:35:28 +0530 Subject: [PATCH 12/15] update the rfc with the latest structure and updates --- 0000-template.md | 2 +- active-rfcs/0000-tables.md | 365 ++++++++++++++++++++++++++----------- 2 files changed, 261 insertions(+), 106 deletions(-) diff --git a/0000-template.md b/0000-template.md index 9645bf9..308b0c0 100644 --- a/0000-template.md +++ b/0000-template.md @@ -33,7 +33,7 @@ Add examples to explain the approach as well. Why should we _not_ do this? Please consider: -- implementation cost, both in term of code size and complexity +- implementation cost, both in terms of code size and complexity - whether the proposed feature can be implemented in user space - the impact on the NPM Package API - integration of this feature with other existing and planned features diff --git a/active-rfcs/0000-tables.md b/active-rfcs/0000-tables.md index 339d148..105daef 100644 --- a/active-rfcs/0000-tables.md +++ b/active-rfcs/0000-tables.md @@ -14,7 +14,7 @@ Tables are one of the most heavily requested features in Excalidraw for past few Tables are nothing but a collection of Text Containers next to each other in form of a grid with `x` rows and `y` columns. -Lets see how the data structure of table element look like. +Let's see how the data structure of table element look like. A `table` element will have `rows` and `columns` defining the grid. The `table` will also have `x` , `y` coordinates for positioning on canvas and also the `width` and `height` for dimensions. @@ -22,57 +22,91 @@ The `table` element consists of multiple cells. Each cell will have an `id`, `x` Below is the sample JSON representation of a `table` element -```js +```json { "id": "table-id", "type": "table", "x": 100, "y": 100, "title": "Table Title", - "rows": 2, - "columns": 2, - "cells": [ - { - "id": "cell-1", - "row": 0, - "column": 0, - "x": 100, - "y": 100, + "cells": { + "cell-11": { + "id": "cell-11", "width": 100, "height": 50, - "boundElements": [{id: "text-1", type: "text"}] + "content": "Cell-11", + "rowId": "row-1", + "columnId": "col-1", + "timestamp": 1633257618000 }, - { - "id": "cell-2", - "x": 200, - "y": 100, + "cell-12": { + "id": "cell-12", "row": 0, "column": 1, "width": 150, "height": 50, - "boundElements": [{id: "text-2", type: "text"}] + "content": "Cell-12", + "rowId": "row-1", + "columnId": "col-2", + "timestamp": 1633257619000 }, - { - "id": "cell-3", - "x": 100, - "y": 150, - "row": 1, - "column": 0, + "cell-21": { + "id": "cell-21", "width": 120, "height": 60, - "boundElements": [{id: "text3", type: "text"}] + "content": "Cell-21", + "rowId": "row-2", + "columnId": "col-1", + "timestamp": 1633257621000 }, - { - "id": "cell-4", - "x": 220, - "y": 150, - "row": 1, - "column": 1, + "cell-22": { + "id": "cell-22", "width": 130, "height": 60, - "boundElements": [{id: "text-4", type: "text"}] + "content": "Cell-22", + "rowId": "row-2", + "columnId": "col-2", + "timestamp": 1633257617000 } - ], + }, + "rows": { + "row-1": { + "id": "row-1", + "index": 0, + "cellIds": [ + "cell-11", + "cell-12" + ], + "lastUpdated": 1633257617000, + "isDeleted": false + }, + "row-2": { + "id": "row-2", + "index": 1, + "cellIds": [ + "cell-21", + "cell-22" + ], + "lastUpdated": 1633257620000, + "isDeleted": false + } + }, + "columns:": { + "column-1": { + "id": "column-1", + "index": 0, + "title": "Column 1", + "timestamp": 1633257617000, + "isDeleted": false + }, + "column-2": { + "id": "column-2", + "index": 1, + "title": "Column 2", + "timestamp": 1633257617000, + "isDeleted": false + } + }, "angle": 0, "strokeColor": "#000000", "backgroundColor": "transparent", @@ -90,26 +124,45 @@ Below is the sample JSON representation of a `table` element "link": null, "locked": false } - ``` +* A table will have a `title` as well `x` and `y` coordinates as well. +* The table consists of `cells` which is an object, defining the content of each cell. These cells are virtual text elements. However if we don't go ahead with virtual text elements then `boundTextElementIds` will be reintroduced. + The `cells` is an object as the earlier approach was to keep it an array but as mentioned in the #alternatives` section, it was not a good idea to keep it as an array for performance reasons. + +### Cell +Each `cell` has the :point_down: attributes +* `id` - to uniquely identify the cell. +* `x` and `y` - to keep track of the position of the cell. +* `content` - the text content of the cell. +* `timestamp` - to keep track of the last update timestamp. This will be used in reconciliation to resolve conflicts when cells are reordered. +* `rowId` and `columnId` - to keep track of the row and column it belongs to. This can be useful for quick lookup for updates, however will be removed if not needed. +* `width` and `height` - to keep track of the dimensions of the cell. This will be used in resizing the cell as each cell can have a different dimension + +### row +Each `row` has the :point_down: attributes +* `id` - to uniquely identify the row. +* `index` - To keep track of the index of the row. This will be used in reordering of rows. +* `cellIds` - which is an array of cell ids. This helps in keeping track of the cells in the row. +* `lastUpdated` - to keep track of the last update timestamp. This will be used in reconciliation to resolve conflicts when rows are reordered. +* `isDeleted` - to keep track of whether the row is deleted or not. + +### column +Each `column` has the :point_down: attributes +* `id` - to uniquely identify the column. +* `index` - To keep track of the index of the column. This will be used in reordering of columns. +* `title` - the name of the column. +* `lastUpdated` - to keep track of the last update timestamp. This will be used in reconciliation to resolve conflicts when columns are reordered. +* `isDeleted` - to keep track of whether the column is deleted or not. -A table will have a `title` as well and the count of `rows` and `columns` as well. - -The table consists of `cells` defining the content of each cell. -Each cell has an `id`, `x`, `y` and dimensions. Since when resizing the width and height of cells can be altered hence recording the dimensions of each cell. - -The `row` and `column` helps in identifying the row and column indexes of the cell. - -Each cell has a `boundElements` to keep it in sync with text containers / labeled arrows, however we can surely rename it to `textId` to keep it simple. ## Adding and Deleting rows / columns To start with we can keep it simple and just allow to add -3 x 3 grid table. Once table is added we can have a add row and add column button to increase the rows and columns similar to how **Miro** has. +3 x 3 grid table. Once table is added we can have "+" button to add row and add column button to increase the rows and columns similar to how **Miro** has. ![uploaded image](https://i.imgur.com/X21LzGR.png) -Similarly and entire `row` and `column` can be removed by right clicking a cell. We just set the attribute `isDeleted` to true for all cells in that row / column and since we are storing the `row` and `column` index in each cell, it will be easy to identify which cells to remove. +Similarly and entire `row` and `column` can be removed by right-clicking a cell. We just set the attribute `isDeleted` to true for all cells in that row / column and since we are storing the `row` and `column` index in each cell, it will be easy to identify which cells to remove. ## Interacting with tables @@ -117,7 +170,7 @@ Similar to other shape tool, the table tool will be resizable, movable etc. Whenever user clicks on the cell - show a text box to updated or add the text. The text will start wrapping as per the cell dimensions similar to how it works for text container. -Additionally if a user clicks on the cell stroke (width / height) should be adjustable. So we need to do a hit test whether the cell stroke is being hit and allow the user to adjust dimensions. +Additionally, if a user clicks on the cell stroke (width / height) should be adjustable. So we need to do a hit test whether the cell stroke is being hit and allow the user to adjust dimensions. But since this would be internally using rectangles so this should be possible without custom code. What we need to support is allow the width / height of all cells in that row /column when any one of them is moved. @@ -127,20 +180,17 @@ Since the table is not just a single element but a collection of different eleme Ideally the `first` `row` and `column` should be preserved for the `row` and `column` `header`. But do we need a separate distinction for headers ? (Eg showing header in diff background color or some highlighter how miro does it). -I think the users will be able to style the headers differently once we have support for wyswyg editor, so till then lets keep it simple. +I think the users will be able to style the headers differently once we have support for wysiwyg editor, so till then lets keep it simple. ## Reordering rows and columns This is a powerful feature and good to have. Users can drag the rows and columns and reorder them on the UI. -However I think in the first release we might not want to support this and add this as an enhancement in future releases. +However, I think in the first release we might not want to support this and add this as an enhancement in future releases. +The `index` attribute in `row` and `column` will be used to resolve conflicts when reordering the rows and columns. ## Alternatives -### Virtual Text Elements - -In the above approach the text elements with `containerId` as `cell id` will exists in the `json`. This means there will be 4 separate `text` elements in the `json`. - -Here is an alternative version - Having `cells` as virtual text elements instead of actual text elements. +### Cells as array of elements ```js { @@ -160,10 +210,7 @@ Here is an alternative version - Having `cells` as virtual text elements instead "y": 100, "width": 100, "height": 50, - "text":"Cell 1", - "fontSize": 16; - "fontFamily": 1; - + "boundElements": [{id: "text-1", type: "text"}] }, { "id": "cell-2", @@ -173,9 +220,7 @@ Here is an alternative version - Having `cells` as virtual text elements instead "column": 1, "width": 150, "height": 50, - "text":"Cell 2", - "fontSize": 16; - "fontFamily": 1; + "boundElements": [{id: "text-2", type: "text"}] }, { "id": "cell-3", @@ -185,9 +230,7 @@ Here is an alternative version - Having `cells` as virtual text elements instead "column": 0, "width": 120, "height": 60, - "text":"Cell 3", - "fontSize": 16; - "fontFamily": 1; + "boundElements": [{id: "text3", type: "text"}] }, { "id": "cell-4", @@ -197,9 +240,7 @@ Here is an alternative version - Having `cells` as virtual text elements instead "column": 1, "width": 130, "height": 60, - "text":"Cell 4", - "fontSize": 16; - "fontFamily": 1; + "boundElements": [{id: "text-4", type: "text"}] } ], "angle": 0, @@ -219,28 +260,29 @@ Here is an alternative version - Having `cells` as virtual text elements instead "link": null, "locked": false } + ``` +A table will have a `title` as well and the count of `rows` and `columns` as well. -As you can see above the `cells` will contain all the attributes (only some of text attributes are shown above) and at the time of rendering these text elements will be drawn, however we won't be storing the text elements as separate elements. -This would simplify the data structure and reduce the hierarchy, thus helping in better maintainance +The table consists of `cells` defining the content of each cell. +Each cell has an `id`, `x`, `y` and dimensions. Since when resizing the width and height of cells can be altered hence recording the dimensions of each cell. -#### Questions +The `row` and `column` helps in identifying the row and column indexes of the cell. -When `cells` are virtual elements which are just rendered on canvas but physically they don't exist as separate text elements? This will definately simplify the data structure, however there are some unknowns listed below which needs to be evaluated before we move with this approach. +Each cell has a `boundElements` to keep it in sync with text containers / labeled arrows, however we can surely rename it to `textId` to keep it simple. -- With above approach of virtual elements, how will impact the overall performance of interacting with tables since we heavily rely on actual elements ? -- Need to verify if this approach is feasable with current state of TextWYSIWYG -- How will this impact collaboration ? We may need a custom logic for tables? +#### `Drawbacks` -Yes we will need write custom logic to handle reconcilation in `tables`, mainly attaching `timestamp` to each cell and writing a custom algorithm to resolve the conflicts and ensuring latest changes are applied to all the collaborators. +This was the `first` approach however this would slow down the operations significantly during lookup since it's an array +hence not going ahead with this structure. -### Storing cells with respect to rows to ease reordering of rows and columns +##### `Storing cells with respect to rows to ease reordering of rows and columns` In the suggested approach, reordering might get difficult as we will have to keep track of which cells should be swapped - lets say if we swap row 1 with row 3, we will have to iterate through all the cells and check which cells have `row` attribute set to `row 1` and swap them with cells with `row` attribute set to `row 3`. Here is an alternate approach to ease the above process. -```js +```json { "id": "table-id", "type": "table", @@ -315,19 +357,19 @@ Here is an alternate approach to ease the above process. ``` -As you can see now all the data related to rows stays together and whenever the rows are swapped, we can swapped the entire row between `source` and `destination` index. +As you can see now all the data related to rows stays together and whenever the rows are swapped, we can swap the entire row between `source` and `destination` index. Whenever the columns are swapped, we will need to swap the cells of the `source` index in each `row` with the `destination` index in each `row`. This way the swapping becomes easier since we need not filter the cells based on row / column positions. -#### Questions +##### Questions -- This approach definately will ease the swapping and handling of rows and columns. How do we track changes when columns or rows are reordered ? +- This approach definitely will ease the swapping and handling of rows and columns. How do we track changes when columns or rows are reordered ? We can add an `index` attribute which helps in maintaining the correct sequence of `rows` and `columns` and `lastUpdated` helps in resolving conflicts in reordering. -```js +```json { "id": "table-id", "type": "table", @@ -408,25 +450,22 @@ We can add an `index` attribute which helps in maintaining the correct sequence } ``` -We will need to write a custom reconcilation to reconcile correctly for this case. +We will need to write a custom reconciliation to reconcile correctly for this case. When there are updates we will compare the `index` and if `index` is updated, it means rows and columns were reordered and compare the `lastUpdated` as well to ensure most recent update is applied to all the collaborators. -Additionally each cell will also have a `timestamp` attribute if we consider virtual text elements (as mentioned in previous approach) to resolve conflicts when content updated on same cell. +Additionally, each cell will also have a `timestamp` attribute if we consider virtual text elements (as mentioned in previous approach) to resolve conflicts when content updated on same cell. -```js +```json { "id": "table-id", "type": "table", "x": 100, "y": 100, "title": "Table Title", - "rows": [{ - "id": "row-1", - "index": 0, - "lastUpdated": 1633257617000 - "cells": [{ - "id": "cell-1", + "cells": { + "cell-11": { + "id": "cell-11", "x": 100, "y": 100, "width": 100, @@ -434,8 +473,8 @@ Additionally each cell will also have a `timestamp` attribute if we consider vir "content": "Cell-11", "timestamp": 1633257618000 }, - { - "id": "cell-2", + "cell-12": { + "id": "cell-12", "x": 200, "y": 100, "row": 0, @@ -444,14 +483,9 @@ Additionally each cell will also have a `timestamp` attribute if we consider vir "height": 50, "content": "Cell-12", "timestamp": 1633257619000 - } - ]}, - { - "id": "row-2", - "index": 1, - "lastUpdated": 1633257620000 - "cells:[{ - "id": "cell-3", + }, + "cell-21":{ + "id": "cell-21", "x": 100, "y": 150, "width": 120, @@ -459,29 +493,44 @@ Additionally each cell will also have a `timestamp` attribute if we consider vir "content": "Cell-21", "timestamp": 1633257621000 }, - { - "id": "cell-4", + "cell-22": { + "id": "cell-22", "x": 220, "y": 150, "width": 130, "height": 60, "content": "Cell-22", "timestamp": 1633257617000 - } - }], - "columns:": [{ + }, + "rows": { + "row-1": { + "id": "row-1", + "index": 0, + "cellIds":["cell-11", "cell-12"], + "lastUpdated": 1633257617000 + }, + "row-2": { + "id": "row-2", + "index": 1, + "cellIds":["cell-21", "cell-22"], + "lastUpdated": 1633257620000, + } + }, + "columns": { + "column-1": { "id": "column-1", "index": 0, "title": "Column 1", "timestamp": 1633257617000 }, - { + "column-2": { "id": "column-2", "index": 1, "title": "Column 2", "timestamp": 1633257617000 - }], + } + }, "angle": 0, "strokeColor": "#000000", "backgroundColor": "transparent", @@ -502,13 +551,119 @@ Additionally each cell will also have a `timestamp` attribute if we consider vir ``` - Will this work for multiplayer undo/redo ? +### Virtual Text Elements + +In the above approach the text elements with `containerId` as `cell id` will exists in the `json`. This means there will be 4 separate `text` elements in the `json`. + +Here is an alternative version - Having `cells` as virtual text elements instead of actual text elements. + +```js +{ + "id": "table-id", + "type": "table", + "x": 100, + "y": 100, + "title": "Table Title", + "rows": 2, + "columns": 2, + "cells": [ + { + "id": "cell-1", + "row": 0, + "column": 0, + "x": 100, + "y": 100, + "width": 100, + "height": 50, + "text":"Cell 1", + "fontSize": 16; + "fontFamily": 1; + + }, + { + "id": "cell-2", + "x": 200, + "y": 100, + "row": 0, + "column": 1, + "width": 150, + "height": 50, + "text":"Cell 2", + "fontSize": 16; + "fontFamily": 1; + }, + { + "id": "cell-3", + "x": 100, + "y": 150, + "row": 1, + "column": 0, + "width": 120, + "height": 60, + "text":"Cell 3", + "fontSize": 16; + "fontFamily": 1; + }, + { + "id": "cell-4", + "x": 220, + "y": 150, + "row": 1, + "column": 1, + "width": 130, + "height": 60, + "text":"Cell 4", + "fontSize": 16; + "fontFamily": 1; + } + ], + "angle": 0, + "strokeColor": "#000000", + "backgroundColor": "transparent", + "fillStyle": "hachure", + "strokeWidth": 1, + "strokeStyle": "solid", + "roughness": 0, + "opacity": 100, + "groupIds": [], + "seed": 1, + "version": 1, + "versionNonce": 1, + "isDeleted": false, + "boundElements": null, + "link": null, + "locked": false +} +``` + +As you can see above the `cells` will contain all the attributes (only some of the text attributes are shown above) and at the time of rendering these text elements will be drawn, however we won't be storing the text elements as separate elements. +This would simplify the data structure and reduce the hierarchy, thus helping in better maintenance. + +#### Questions + +1. When `cells` are virtual elements which are just rendered on canvas, but physically they don't exist as separate text elements? This will definately simplify the data structure, however there are some unknowns listed below which needs to be evaluated before we move with this approach. + + - With above approach of virtual elements, how will impact the overall performance of interacting with tables since we heavily rely on actual elements ? + - Need to verify if this approach is feasible with current state of TextWYSIWYG + - How will this impact collaboration ? We may need a custom logic for tables? + + Yes we will need write custom logic to handle reconciliation in `tables`, mainly attaching `timestamp` to each cell and writing a custom algorithm to resolve the conflicts and ensuring latest changes are applied to all the collaborators. + +2. Do we need `isDeleted` for each cell ? + + - Not needed as we can't just remove an individual cell, instead either an entire `row` or `column` can be removed. + +3. Do we need per property syncing? + + - May be, to ensure that we don't ignore the changes made by other collaborators e.g.- if a user A made a text change and a user B made a dimension change to the same cell, we need to ensure that both changes are applied. # Adoption strategy - If we implement this proposal, how will existing Excalidraw users adopt it? -This is a new feature so adoption should be straight forward and they will be using it as needed. +This is a new feature so adoption should be straight forward, and they will be using it as needed. - Is this a breaking change? If yes how are migrating the existing Excalidraw users ? No this is not a breaking change (unless we change some existing implementation) so hopefully no migration will be needed. +However for virtual text elements some existing code might need to be updated with the way wysiwyg editor is being used and hence that could introduce some changes we need to test out before pushing this out. From a86453723999b40a42ade3b293d84dd4045067c9 Mon Sep 17 00:00:00 2001 From: Aakansha Doshi Date: Fri, 2 Aug 2024 16:30:35 +0530 Subject: [PATCH 13/15] uplift width and height to respective column and row attributes --- active-rfcs/0000-tables.md | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/active-rfcs/0000-tables.md b/active-rfcs/0000-tables.md index 105daef..5db8dc7 100644 --- a/active-rfcs/0000-tables.md +++ b/active-rfcs/0000-tables.md @@ -32,8 +32,6 @@ Below is the sample JSON representation of a `table` element "cells": { "cell-11": { "id": "cell-11", - "width": 100, - "height": 50, "content": "Cell-11", "rowId": "row-1", "columnId": "col-1", @@ -43,8 +41,6 @@ Below is the sample JSON representation of a `table` element "id": "cell-12", "row": 0, "column": 1, - "width": 150, - "height": 50, "content": "Cell-12", "rowId": "row-1", "columnId": "col-2", @@ -52,8 +48,6 @@ Below is the sample JSON representation of a `table` element }, "cell-21": { "id": "cell-21", - "width": 120, - "height": 60, "content": "Cell-21", "rowId": "row-2", "columnId": "col-1", @@ -78,7 +72,8 @@ Below is the sample JSON representation of a `table` element "cell-12" ], "lastUpdated": 1633257617000, - "isDeleted": false + "isDeleted": false, + "height": 50 }, "row-2": { "id": "row-2", @@ -88,7 +83,8 @@ Below is the sample JSON representation of a `table` element "cell-22" ], "lastUpdated": 1633257620000, - "isDeleted": false + "isDeleted": false, + "height": 60 } }, "columns:": { @@ -97,14 +93,16 @@ Below is the sample JSON representation of a `table` element "index": 0, "title": "Column 1", "timestamp": 1633257617000, - "isDeleted": false + "isDeleted": false, + "width": 100 }, "column-2": { "id": "column-2", "index": 1, "title": "Column 2", "timestamp": 1633257617000, - "isDeleted": false + "isDeleted": false, + "width": 150 } }, "angle": 0, @@ -136,7 +134,6 @@ Each `cell` has the :point_down: attributes * `content` - the text content of the cell. * `timestamp` - to keep track of the last update timestamp. This will be used in reconciliation to resolve conflicts when cells are reordered. * `rowId` and `columnId` - to keep track of the row and column it belongs to. This can be useful for quick lookup for updates, however will be removed if not needed. -* `width` and `height` - to keep track of the dimensions of the cell. This will be used in resizing the cell as each cell can have a different dimension ### row Each `row` has the :point_down: attributes @@ -145,6 +142,7 @@ Each `row` has the :point_down: attributes * `cellIds` - which is an array of cell ids. This helps in keeping track of the cells in the row. * `lastUpdated` - to keep track of the last update timestamp. This will be used in reconciliation to resolve conflicts when rows are reordered. * `isDeleted` - to keep track of whether the row is deleted or not. +* `height` - to keep track of the height of the row. ### column Each `column` has the :point_down: attributes @@ -153,6 +151,7 @@ Each `column` has the :point_down: attributes * `title` - the name of the column. * `lastUpdated` - to keep track of the last update timestamp. This will be used in reconciliation to resolve conflicts when columns are reordered. * `isDeleted` - to keep track of whether the column is deleted or not. +* `width` - to keep track of the width of the column. ## Adding and Deleting rows / columns @@ -656,6 +655,11 @@ This would simplify the data structure and reduce the hierarchy, thus helping in 3. Do we need per property syncing? - May be, to ensure that we don't ignore the changes made by other collaborators e.g.- if a user A made a text change and a user B made a dimension change to the same cell, we need to ensure that both changes are applied. +4. Do we need `width` and `height` for each cell? + + - Each cell can have different dimensions but when a dimension is updated, lets say if cell at (1,2) has increased width by `x`, this means the entire column width gets impacted by same amount `x` + - Similarly, if cell at (1,2) has increased height by `y`, this means the entire row height gets impacted by same amount `y` + - This means that `width` and `height` could be uplifted to the respective `column` and `row` attributes. # Adoption strategy From 256f31875e5f610a0ac031ddb4fd5829d480404e Mon Sep 17 00:00:00 2001 From: Aakansha Doshi Date: Fri, 2 Aug 2024 17:41:31 +0530 Subject: [PATCH 14/15] use for each row/column and cell and combine with top level versionNonce for resolving conflicts --- active-rfcs/0000-tables.md | 188 ++++++++++++------------------------- 1 file changed, 62 insertions(+), 126 deletions(-) diff --git a/active-rfcs/0000-tables.md b/active-rfcs/0000-tables.md index 5db8dc7..cee994a 100644 --- a/active-rfcs/0000-tables.md +++ b/active-rfcs/0000-tables.md @@ -34,8 +34,8 @@ Below is the sample JSON representation of a `table` element "id": "cell-11", "content": "Cell-11", "rowId": "row-1", - "columnId": "col-1", - "timestamp": 1633257618000 + "columnId": "col-1", + "version": 1 }, "cell-12": { "id": "cell-12", @@ -43,15 +43,15 @@ Below is the sample JSON representation of a `table` element "column": 1, "content": "Cell-12", "rowId": "row-1", - "columnId": "col-2", - "timestamp": 1633257619000 + "columnId": "col-2", + "version": 1 }, "cell-21": { "id": "cell-21", "content": "Cell-21", "rowId": "row-2", "columnId": "col-1", - "timestamp": 1633257621000 + "version": 1 }, "cell-22": { "id": "cell-22", @@ -59,8 +59,8 @@ Below is the sample JSON representation of a `table` element "height": 60, "content": "Cell-22", "rowId": "row-2", - "columnId": "col-2", - "timestamp": 1633257617000 + "columnId": "col-2", + "version": 1 } }, "rows": { @@ -70,8 +70,8 @@ Below is the sample JSON representation of a `table` element "cellIds": [ "cell-11", "cell-12" - ], - "lastUpdated": 1633257617000, + ], + "version": 1, "isDeleted": false, "height": 50 }, @@ -82,7 +82,7 @@ Below is the sample JSON representation of a `table` element "cell-21", "cell-22" ], - "lastUpdated": 1633257620000, + "version": 2, "isDeleted": false, "height": 60 } @@ -92,7 +92,7 @@ Below is the sample JSON representation of a `table` element "id": "column-1", "index": 0, "title": "Column 1", - "timestamp": 1633257617000, + "version": 2, "isDeleted": false, "width": 100 }, @@ -100,7 +100,7 @@ Below is the sample JSON representation of a `table` element "id": "column-2", "index": 1, "title": "Column 2", - "timestamp": 1633257617000, + "version": 1, "isDeleted": false, "width": 150 } @@ -115,8 +115,8 @@ Below is the sample JSON representation of a `table` element "opacity": 100, "groupIds": [], "seed": 1, - "version": 1, - "versionNonce": 1, + "version": 2, + "versionNonce": 19289929, "isDeleted": false, "boundElements": null, "link": null, @@ -124,15 +124,15 @@ Below is the sample JSON representation of a `table` element } ``` * A table will have a `title` as well `x` and `y` coordinates as well. -* The table consists of `cells` which is an object, defining the content of each cell. These cells are virtual text elements. However if we don't go ahead with virtual text elements then `boundTextElementIds` will be reintroduced. +* The table consists of `cells` which is an object, defining the content of each cell. These cells are virtual text elements. However, if we don't go ahead with virtual text elements then `boundTextElementIds` will be reintroduced. The `cells` is an object as the earlier approach was to keep it an array but as mentioned in the #alternatives` section, it was not a good idea to keep it as an array for performance reasons. -### Cell +### cell Each `cell` has the :point_down: attributes * `id` - to uniquely identify the cell. * `x` and `y` - to keep track of the position of the cell. * `content` - the text content of the cell. -* `timestamp` - to keep track of the last update timestamp. This will be used in reconciliation to resolve conflicts when cells are reordered. +* `version` - to keep track of the version of the cell. This will be used in reconciliation to resolve conflicts when cells are updated. * `rowId` and `columnId` - to keep track of the row and column it belongs to. This can be useful for quick lookup for updates, however will be removed if not needed. ### row @@ -140,7 +140,7 @@ Each `row` has the :point_down: attributes * `id` - to uniquely identify the row. * `index` - To keep track of the index of the row. This will be used in reordering of rows. * `cellIds` - which is an array of cell ids. This helps in keeping track of the cells in the row. -* `lastUpdated` - to keep track of the last update timestamp. This will be used in reconciliation to resolve conflicts when rows are reordered. +* `version` - to keep track of the version of the row. This will be used in reconciliation to resolve conflicts when rows are updated / reordered. * `isDeleted` - to keep track of whether the row is deleted or not. * `height` - to keep track of the height of the row. @@ -149,10 +149,12 @@ Each `column` has the :point_down: attributes * `id` - to uniquely identify the column. * `index` - To keep track of the index of the column. This will be used in reordering of columns. * `title` - the name of the column. -* `lastUpdated` - to keep track of the last update timestamp. This will be used in reconciliation to resolve conflicts when columns are reordered. +* `version` - to keep track of the version of the column. This will be used in reconciliation to resolve conflicts when columns are updated / reordered. * `isDeleted` - to keep track of whether the column is deleted or not. * `width` - to keep track of the width of the column. +- When resolving conflicts, we give preference to the client having highest `version`. +- In case multiple clients end up having the same `version`, we check which client has highest `versionNonce` and apply their updates. ## Adding and Deleting rows / columns @@ -187,9 +189,9 @@ This is a powerful feature and good to have. Users can drag the rows and columns However, I think in the first release we might not want to support this and add this as an enhancement in future releases. The `index` attribute in `row` and `column` will be used to resolve conflicts when reordering the rows and columns. -## Alternatives +# Alternatives -### Cells as array of elements +## Cells as array of elements ```js { @@ -270,12 +272,12 @@ The `row` and `column` helps in identifying the row and column indexes of the ce Each cell has a `boundElements` to keep it in sync with text containers / labeled arrows, however we can surely rename it to `textId` to keep it simple. -#### `Drawbacks` +### `Drawbacks` This was the `first` approach however this would slow down the operations significantly during lookup since it's an array hence not going ahead with this structure. -##### `Storing cells with respect to rows to ease reordering of rows and columns` +## `Storing cells with respect to rows to ease reordering of rows and columns` In the suggested approach, reordering might get difficult as we will have to keep track of which cells should be swapped - lets say if we swap row 1 with row 3, we will have to iterate through all the cells and check which cells have `row` attribute set to `row 1` and swap them with cells with `row` attribute set to `row 3`. @@ -296,7 +298,7 @@ Here is an alternate approach to ease the above process. "y": 100, "width": 100, "height": 50, - "boundElements": [{id: "text-1", type: "text"}] + "boundElements": [{"id": "text-1", "type": "text"}] }, { "id": "cell-2", @@ -306,18 +308,18 @@ Here is an alternate approach to ease the above process. "column": 1, "width": 150, "height": 50, - "boundElements": [{id: "text-2", type: "text"}] + "boundElements": [{"id": "text-2", "type": "text"}] } ]}, { "id": "row-2", - "cells:[{ + "cells":[{ "id": "cell-3", "x": 100, "y": 150, "width": 120, "height": 60, - "boundElements": [{id: "text3", type: "text"}] + "boundElements": [{"id": "text3", "type": "text"}] }, { "id": "cell-4", @@ -325,7 +327,7 @@ Here is an alternate approach to ease the above process. "y": 150, "width": 130, "height": 60, - "boundElements": [{id: "text-4", type: "text"}] + "boundElements": [{"id": "text-4", "type": "text"}] } }], "columns:": [{ @@ -362,7 +364,7 @@ Whenever the columns are swapped, we will need to swap the cells of the `source` This way the swapping becomes easier since we need not filter the cells based on row / column positions. -##### Questions +### Questions - This approach definitely will ease the swapping and handling of rows and columns. How do we track changes when columns or rows are reordered ? @@ -385,7 +387,7 @@ We can add an `index` attribute which helps in maintaining the correct sequence "y": 100, "width": 100, "height": 50, - "boundElements": [{id: "text-1", type: "text"}], + "boundElements": [{"id": "text-1", "type": "text"}], }, { "id": "cell-2", @@ -395,20 +397,20 @@ We can add an `index` attribute which helps in maintaining the correct sequence "column": 1, "width": 150, "height": 50, - "boundElements": [{id: "text-2", type: "text"}], + "boundElements": [{"id": "text-2", "type": "text"}] } ]}, { "id": "row-2", "index": 1, - "lastUpdated": 1633257617000 - "cells:[{ + "lastUpdated": 1633257617000, + "cells":[{ "id": "cell-3", "x": 100, "y": 150, "width": 120, "height": 60, - "boundElements": [{id: "text3", type: "text"}], + "boundElements": [{"id": "text3", "type": "text"}] }, { "id": "cell-4", @@ -416,8 +418,7 @@ We can add an `index` attribute which helps in maintaining the correct sequence "y": 150, "width": 130, "height": 60, - "boundElements": [{id: "text-4", type: "text"}], - + "boundElements": [{"id": "text-4", "type": "text"}] } }], "columns:": [{ @@ -550,103 +551,21 @@ Additionally, each cell will also have a `timestamp` attribute if we consider vi ``` - Will this work for multiplayer undo/redo ? -### Virtual Text Elements - -In the above approach the text elements with `containerId` as `cell id` will exists in the `json`. This means there will be 4 separate `text` elements in the `json`. -Here is an alternative version - Having `cells` as virtual text elements instead of actual text elements. - -```js -{ - "id": "table-id", - "type": "table", - "x": 100, - "y": 100, - "title": "Table Title", - "rows": 2, - "columns": 2, - "cells": [ - { - "id": "cell-1", - "row": 0, - "column": 0, - "x": 100, - "y": 100, - "width": 100, - "height": 50, - "text":"Cell 1", - "fontSize": 16; - "fontFamily": 1; - - }, - { - "id": "cell-2", - "x": 200, - "y": 100, - "row": 0, - "column": 1, - "width": 150, - "height": 50, - "text":"Cell 2", - "fontSize": 16; - "fontFamily": 1; - }, - { - "id": "cell-3", - "x": 100, - "y": 150, - "row": 1, - "column": 0, - "width": 120, - "height": 60, - "text":"Cell 3", - "fontSize": 16; - "fontFamily": 1; - }, - { - "id": "cell-4", - "x": 220, - "y": 150, - "row": 1, - "column": 1, - "width": 130, - "height": 60, - "text":"Cell 4", - "fontSize": 16; - "fontFamily": 1; - } - ], - "angle": 0, - "strokeColor": "#000000", - "backgroundColor": "transparent", - "fillStyle": "hachure", - "strokeWidth": 1, - "strokeStyle": "solid", - "roughness": 0, - "opacity": 100, - "groupIds": [], - "seed": 1, - "version": 1, - "versionNonce": 1, - "isDeleted": false, - "boundElements": null, - "link": null, - "locked": false -} -``` +### `Drawbacks` -As you can see above the `cells` will contain all the attributes (only some of the text attributes are shown above) and at the time of rendering these text elements will be drawn, however we won't be storing the text elements as separate elements. -This would simplify the data structure and reduce the hierarchy, thus helping in better maintenance. +This would slow down the operations significantly during lookup since it's still an array +hence not going ahead with this structure. -#### Questions +# Questions -1. When `cells` are virtual elements which are just rendered on canvas, but physically they don't exist as separate text elements? This will definately simplify the data structure, however there are some unknowns listed below which needs to be evaluated before we move with this approach. +1. If `cells` are virtual elements which are just rendered on canvas, but physically they don't exist as separate text elements. This will definitely simplify the data structure, however there are some unknowns listed below which needs to be evaluated before we move with this approach. - With above approach of virtual elements, how will impact the overall performance of interacting with tables since we heavily rely on actual elements ? - Need to verify if this approach is feasible with current state of TextWYSIWYG - How will this impact collaboration ? We may need a custom logic for tables? - Yes we will need write custom logic to handle reconciliation in `tables`, mainly attaching `timestamp` to each cell and writing a custom algorithm to resolve the conflicts and ensuring latest changes are applied to all the collaborators. + Yes we will need write custom logic to handle reconciliation in `tables`, mainly attaching `version` to each row/column and cell and using a combination with `versionNonce` as explained earlier and writing a custom algorithm to resolve the conflicts and ensuring latest changes are applied to all the collaborators. 2. Do we need `isDeleted` for each cell ? @@ -654,12 +573,29 @@ This would simplify the data structure and reduce the hierarchy, thus helping in 3. Do we need per property syncing? - - May be, to ensure that we don't ignore the changes made by other collaborators e.g.- if a user A made a text change and a user B made a dimension change to the same cell, we need to ensure that both changes are applied. + - Yes we will need, to ensure that we don't ignore the changes made by other collaborators when cell properties updated eg font size, font family and later when we have support for wysiwyg editor, we need to ensure that the changes are in sync with all the collaborators. + - However, for `tables` we might ignore text properties syncing and implement it later. + - 4. Do we need `width` and `height` for each cell? - Each cell can have different dimensions but when a dimension is updated, lets say if cell at (1,2) has increased width by `x`, this means the entire column width gets impacted by same amount `x` - Similarly, if cell at (1,2) has increased height by `y`, this means the entire row height gets impacted by same amount `y` - This means that `width` and `height` could be uplifted to the respective `column` and `row` attributes. +5. Are timestamps reliable for reconciliation ? + + - No, timestamps if generated on client are definitely not reliable for several factors, some of them mentioned :point_down: + - Clock drifts - Different client may have different system times leading to inconsistencies + - Timezone differences - Different clients may have different timezones leading to inconsistencies + - Network latency - Different clients may have different network latencies leading to inconsistencies + Hence client generated timestamps are not reliable for reconciliation. However, if we use server generated timestamps, they can also have network latency as well so not reliable as well. + Hence, the way we do it now in production using `version` and `versionNonce` looks like the best way to handle reconciliation. + - `version` - is the version of the element, and it increments for every update + - `versionNonce` - is the nonce (random number) which is generated for every update + Here is how it works + - When a client updates an element, it increments the `version` and generates a new `versionNonce` for that element + - When the client receives an update from the server, it compares the local `version` and the remote elements `version`, whichever is higher gets preference + - If the `version` is same, then it compares the `versionNonce`, whichever is higher gets preference + - In case of `version` being same, we end up randomly choosing the winner but that's ok as it works fine # Adoption strategy @@ -670,4 +606,4 @@ This is a new feature so adoption should be straight forward, and they will be u - Is this a breaking change? If yes how are migrating the existing Excalidraw users ? No this is not a breaking change (unless we change some existing implementation) so hopefully no migration will be needed. -However for virtual text elements some existing code might need to be updated with the way wysiwyg editor is being used and hence that could introduce some changes we need to test out before pushing this out. +However, for virtual text elements some existing code might need to be updated with the way wysiwyg editor is being used and hence that could introduce some changes we need to test out before pushing this out. From 31d7c7edf15315e4ee7b97abc6a19a053d20acb7 Mon Sep 17 00:00:00 2001 From: Aakansha Doshi Date: Fri, 2 Aug 2024 17:45:22 +0530 Subject: [PATCH 15/15] fix typos --- active-rfcs/0000-tables.md | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/active-rfcs/0000-tables.md b/active-rfcs/0000-tables.md index cee994a..e4d8d06 100644 --- a/active-rfcs/0000-tables.md +++ b/active-rfcs/0000-tables.md @@ -193,7 +193,7 @@ The `index` attribute in `row` and `column` will be used to resolve conflicts wh ## Cells as array of elements -```js +```json { "id": "table-id", "type": "table", @@ -211,7 +211,7 @@ The `index` attribute in `row` and `column` will be used to resolve conflicts wh "y": 100, "width": 100, "height": 50, - "boundElements": [{id: "text-1", type: "text"}] + "boundElements": [{"id": "text-1", "type": "text"}] }, { "id": "cell-2", @@ -221,7 +221,7 @@ The `index` attribute in `row` and `column` will be used to resolve conflicts wh "column": 1, "width": 150, "height": 50, - "boundElements": [{id: "text-2", type: "text"}] + "boundElements": [{"id": "text-2", "type": "text"}] }, { "id": "cell-3", @@ -231,7 +231,7 @@ The `index` attribute in `row` and `column` will be used to resolve conflicts wh "column": 0, "width": 120, "height": 60, - "boundElements": [{id: "text3", type: "text"}] + "boundElements": [{"id": "text3", "type": "text"}] }, { "id": "cell-4", @@ -241,7 +241,7 @@ The `index` attribute in `row` and `column` will be used to resolve conflicts wh "column": 1, "width": 130, "height": 60, - "boundElements": [{id: "text-4", type: "text"}] + "boundElements": [{"id": "text-4", "type": "text"}] } ], "angle": 0, @@ -332,11 +332,11 @@ Here is an alternate approach to ease the above process. }], "columns:": [{ "id": "column-1", - "title": "Column 1", + "title": "Column 1" }, { "id": "column-2", - "title": "Column 2", + "title": "Column 2" }], "angle": 0, "strokeColor": "#000000", @@ -380,7 +380,7 @@ We can add an `index` attribute which helps in maintaining the correct sequence "rows": [{ "id": "row-1", "index": 0, - "lastUpdated": 1633257617000 + "lastUpdated": 1633257617000, "cells": [{ "id": "cell-1", "x": 100, @@ -424,12 +424,12 @@ We can add an `index` attribute which helps in maintaining the correct sequence "columns:": [{ "id": "column-1", "index": 0, - "title": "Column 1", + "title": "Column 1" }, { "id": "column-2", "index": 1, - "title": "Column 2", + "title": "Column 2" }], "angle": 0, "strokeColor": "#000000", @@ -588,9 +588,11 @@ hence not going ahead with this structure. - Timezone differences - Different clients may have different timezones leading to inconsistencies - Network latency - Different clients may have different network latencies leading to inconsistencies Hence client generated timestamps are not reliable for reconciliation. However, if we use server generated timestamps, they can also have network latency as well so not reliable as well. + Hence, the way we do it now in production using `version` and `versionNonce` looks like the best way to handle reconciliation. - `version` - is the version of the element, and it increments for every update - `versionNonce` - is the nonce (random number) which is generated for every update + Here is how it works - When a client updates an element, it increments the `version` and generates a new `versionNonce` for that element - When the client receives an update from the server, it compares the local `version` and the remote elements `version`, whichever is higher gets preference