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.

Sign up for GitHub

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

Jump to bottom

[8.0.0.-beta2-rev2] NestedRows: we are able to remove a parent - but only if it's not the last one. #6860

Closed
AMBudnik opened this issue May 11, 2020 · 3 comments
Closed

[8.0.0.-beta2-rev2] NestedRows: we are able to remove a parent - but only if it's not the last one. #6860

AMBudnik opened this issue May 11, 2020 · 3 comments
Assignees
@jansiegel
Labels
bug Nested rows Plugin Regression Issues that were created while adding new changes to the source code

Comments

@AMBudnik
Copy link
Contributor

Description

Issue extracted from #6433 > Case 2.
The following issue contained too many examples so I decided to split those which are solved from those which aren't solved.

Steps to reproduce

  1. Choose 'Remove row' from the context menu for Best Metal Performance.
  2. Undo

Result

  • you cannot undo
  • you get a console error
    Zrzut ekranu 2020-05-11 o 13 52 06

It's a regression case in 8.0.0.-beta1 we did not get an error and some rows were back.

beta1

Demo

https://jsfiddle.net/k5nav2rw/ (8.0.0.-beta2-rev2)

Additional info

  • breaks only when the last parent is removed

Your environment

  • Handsontable version: 8.0.0.-beta2-rev2
  • Browser Name and version: Chrome 80
  • Operating System: macOS Catalina
@AMBudnik AMBudnik added bug Nested rows Plugin Regression Issues that were created while adding new changes to the source code labels May 11, 2020
@AMBudnik AMBudnik mentioned this issue May 11, 2020
Closed
@aninde aninde mentioned this issue May 11, 2020
Closed
@wszymanski wszymanski assigned jansiegel and unassigned wszymanski May 12, 2020
@wszymanski
Copy link
Contributor

May be related to the #6678.

jansiegel added a commit that referenced this issue May 14, 2020
@jansiegel
- Change the dataMap's 'spliceData' method to accept elements as rest…
0965bfd 
… arguments, accordingly to the native 'splice' method

- Rewrite the 'createRow' loop to update the index mappers before calling the 'beforeDataSplice' hook
- Modify the NestedRows cache to work on a pre-cached version of data (workaround for 'getSourceData' no longer returning references to data)
- Change the NestedRows' cache update to update with a pre-cached data, for the same reason as above.
#6860 #6855
@jansiegel jansiegel mentioned this issue May 14, 2020
Merged
1 task
jansiegel added a commit that referenced this issue May 19, 2020
@jansiegel
Post CR changes. #6860
73e26d6
jansiegel added a commit that referenced this issue May 21, 2020
@jansiegel
- Remove the 'beforeDataSplice' hook docs (because of the discussion in
49a1b11 
#6889).

- Refactor some of the PR's code.
- Fix an issue with row creation.�
- Add a test case for 'updateSettings'.
 #6860
jansiegel added a commit that referenced this issue May 21, 2020
@jansiegel
- Post-CR changes
d7f8aa1 
- Fix a problem with adding children to the nested structure, when the reference to the data has been broken
- Add tests to the NestedRows plugin
 #6860
wszymanski added a commit that referenced this issue May 21, 2020
@wszymanski
Just more specific tests #6860
d7d2992
jansiegel added a commit that referenced this issue May 21, 2020
@jansiegel @wszymanski
Update nestedRows and dataMap to work with 8.0-beta2 (#6889)
ed20102 
* - Change the dataMap's 'spliceData' method to accept elements as rest arguments, accordingly to the native 'splice' method
- Rewrite the 'createRow' loop to update the index mappers before calling the 'beforeDataSplice' hook
- Modify the NestedRows cache to work on a pre-cached version of data (workaround for 'getSourceData' no longer returning references to data)
- Change the NestedRows' cache update to update with a pre-cached data, for the same reason as above.
#6860 #6855

* Post CR changes. #6860

* - Remove the 'beforeDataSplice' hook docs (because of the discussion in #6889).
- Refactor some of the PR's code.
- Fix an issue with row creation.�
- Add a test case for 'updateSettings'.
 #6860

* - Post-CR changes
- Fix a problem with adding children to the nested structure, when the reference to the data has been broken
- Add tests to the NestedRows plugin
 #6860

* Just more specific tests #6860

Co-authored-by: Wojciech Szymanski <141330+wszymanski@users.noreply.github.com>
wszymanski added a commit that referenced this issue May 25, 2020
@wszymanski
Build revision 8 of the branch for tests. Fixes: #6916 (problem with …
945d49d 
…focus), #6917 (the ManualRowMove problems), problems with the NestedRows plugin (#6890, #6860, #6855, #6898)
@aninde
Copy link
Contributor

aninde commented May 26, 2020

This bug is fixed with 8.0.0.-beta2-rev8, there's no error in console and parent, a removed row is back, but without their children rows. Is that a result that we want to achieve?

6860-rev8
https://jsfiddle.net/aninde/ykdnh710/
https://jsfiddle.net/aninde/th2uxok4/ - both v8.0.0.-beta2-rev8

@aninde aninde mentioned this issue May 27, 2020
Closed
@aninde
Copy link
Contributor

aninde commented Jul 13, 2020

This issue is still fixed with 8.0.0-beta2-rev22 https://jsfiddle.net/aninde/xfmgeq3o/ so I'm closing it.

@aninde aninde closed this as completed Jul 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jansiegel jansiegel

Labels
bug Nested rows Plugin Regression Issues that were created while adding new changes to the source code
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@wszymanski @jansiegel @AMBudnik @aninde