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

Working solution for adding rows and columns at the expected location #5

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bryvin
Copy link
Contributor

@bryvin bryvin commented Mar 5, 2019

This is source code only, had issues with the package.json and conflicting versions of node modules from when this was last published (and not having them installed).

In reference to: standardnotes/forum#474

@moughxyz
Copy link
Member

moughxyz commented Mar 6, 2019

Oo, nice. What do you mean by source code only? Also, if you haven't already, can you test this using the "Crypto asset summary" demo sheet at standardnotes.org/demo? Just want to make sure it's tested with a sufficiently complex worksheet which relies on cell calculations, etc.

@bryvin
Copy link
Contributor Author

bryvin commented Mar 6, 2019

** Removed content..

I just re-read my post and noticed it did actually break the calculations. RIP... That may actually be tricky to do this then lol..

I'll have to find some more time to go over why it wasn't working with the original version cause it does work in their demos.

@moughxyz
Copy link
Member

moughxyz commented Mar 7, 2019

Ah damn, close one. I figured the calculations would make this difficult. I am curious too why it works in theirs and not ours, and what the original reason for the custom handling on our part was 🤔

@bryvin
Copy link
Contributor Author

bryvin commented Mar 8, 2019

I updated my pull request to have an actual working solution. Again it's only the 1 source file and not the compiled dist due to the npm modules.

I hijacked the render method to make this work. It wasn't working previously because in the event you had handled you were taking the JSON and then using that data to re-save it at the end of the same function. The data however included the old indexes and information, just with updated row/column count. Essentially it was overwriting the legit changes KendoUI was doing with the old data and 1 more row/column.

@moughxyz
Copy link
Member

moughxyz commented Mar 8, 2019

Very nice, I'll test this out sometime in the next few days.

@moughxyz
Copy link
Member

Need a little more time on this, sorry. Should have known better than to say next few days..

@bryvin
Copy link
Contributor Author

bryvin commented Mar 15, 2019

No worries, take your time. 👍

@moughxyz
Copy link
Member

I'm seeing a strange issue with this:

  1. Start a new sheet and type "Foo" in A1 and "Bar" in B1.
  2. Go to the last row, 75, and insert a row below or above.
  3. Any content in row 1 after A1 is deleted. In this case, "Bar" is deleted. If you had content in C1 and D1, it also would be deleted.

This issue does not happen in the production version. But besides that, the new functionality is very nice to have and works as you'd expect.

@twoprops
Copy link

twoprops commented Sep 7, 2019

Hi! The ability to add and delete rows and columns appears to still be broken in the current version (1.3.3) six months after the last comment. As this is core functionality for any spreadsheet (since, oh, VisiCALC) I feel that it is disrespectful to users to list this with other editors and extensions that actually work. SN users are likely to install the editor and put significant effort into making it work, only to discover that critical core functionality is broken. I suggest amending the editor description within SN to warn users that this is not currently a fully functioning editor.

@johnny243 johnny243 changed the base branch from master to main February 22, 2021 01:35
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.

3 participants