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

[8.0.0-beta2-rev8] Undoing removal of nestedrows when all collapsed parents are selected don't work #6941

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

Comments

@aninde
Copy link
Contributor

aninde commented May 27, 2020

Description

When all collapsed parents rows are selected, removing all of them by contextMenu can not be undone.
In 8.0.0-rev8 only corner left after that, with is a regression for 7.4.2, where colHeaders stay with the corner.

Expected result: we want to be able to delete rows and undo this operation.
The question remains whether we want nested children-rows to return after the undo.

Case 1

  1. Collapse each parent row.
  2. Select all of them by dragging the selection.
  3. Remove rows by contextMenu

Case 2

  1. Collapse each parent row.
  2. Select all of them by clicking and holding rowHeaders.
  3. Remove rows by contextMenu

Actual result of all cases in 7.4.2, 1.14.0
colHeaders left, cannot undo.
Screenshot 2020-05-27 at 12 11 05

Actual result of all cases in 8.0.0-beta2-rev.8
Only corner left, cannot undo.
Screenshot 2020-05-27 at 12 12 13

Actual result 8.0.0-beta1:
Nothing happens + the same error in console
handsontable.full.js:108164 Uncaught TypeError: Cannot read property '__children' of undefined
handsontable.full.js:108164
chrome-case3

Demo

https://jsfiddle.net/wojciech/j4fs38oz - vbeta1 (broken worse, with error in console)
https://jsfiddle.net/aninde/9q5hjobm/1/ - v8.0.0-beta2-rev1 (broken)
https://jsfiddle.net/572Lefpz/ - v7.4.2 (partially fixed, cannot undo)
http://jsfiddle.net/aninde/65ur7aLb/ - v1.14.0 (partially fixed, colHearders stays, cannot undo)
http://jsfiddle.net/aninde/ekLrxap4/ v8.0.0-beta2-rev8 (partially fixed, can not undo, only corner left)

Your environment

  • Handsontable version: 8.0.0-beta1 (the worst), v1.14.0 can not undo
  • Browser Name and version: Firefox, Chrome
  • Operating System: macOs Catalina
@aninde
Copy link
Contributor Author

aninde commented May 27, 2020

Can be related to #6890

@aninde aninde changed the title [8.0.0-beta1] Removing nestedrows when all collapsed parents are selected don't work [8.0.0-beta1] Removing and undo of nestedrows when all collapsed parents are selected don't work May 27, 2020
@aninde aninde changed the title [8.0.0-beta1] Removing and undo of nestedrows when all collapsed parents are selected don't work [8.0.0-beta2-rev8] Undoing removal of nestedrows when all collapsed parents are selected don't work May 27, 2020
@aninde
Copy link
Contributor Author

aninde commented May 28, 2020

8.0.0.-beta2-rev-10 still got same bug https://jsfiddle.net/aninde/xmgvnyj5/

@jansiegel
Copy link
Member

If the only difference between 7.4.2 and the current version is the fact that there are no column headers left (which I think is intentional: #6947 (comment)), and undo doesn't work in both cases, is this really a regression? @aninde @wojciechczerniak

@wojciechczerniak
Copy link
Contributor

wojciechczerniak commented Jun 3, 2020

@aninde As @jansiegel mentioned, the difference in headers display is intentional.

@jansiegel Undo/redo issue might be a regression? But I think this issue is the same @budnix already solved in #6947. When we remove all rows the columns are not restored. It should be fixed on develop so you can't reproduce it anymore?

@AMBudnik
Copy link
Contributor

AMBudnik commented Jun 3, 2020

Here's a demo made out of the latest commit on the develop branch http://jsfiddle.net/AMBudnik/47xyb90m/ and with @aninde 's scenario it breaks the same way.

issue

It is broken really bad, as any interaction with the table throws the same error.

@wojciechczerniak
Copy link
Contributor

wojciechczerniak commented Jun 3, 2020

@AMBudnik latest commit on the develop branch doesn't have /dist rebuild. You're checking version 7.2.2 with the buildDate: 07/11/2019 19:18:08

@AMBudnik
Copy link
Contributor

AMBudnik commented Jun 3, 2020

You're right @wojciechczerniak I guess I got too excited about solving this one... 🙈

Here's the proper demo http://jsfiddle.net/AMBudnik/o1v0z3af/

Reproduction
ok

We do not get any console errors, we're able to get back all the parents with undo.

@jansiegel
Copy link
Member

@AMBudnik So basically it works like 7.4.2 and we can close this one, right?

@aninde
Copy link
Contributor Author

aninde commented Jun 3, 2020

@jansiegel true, the regression error is that after removing only the corner remains. However, this is planned, so I think we can close this issue.

@aninde aninde closed this as completed Jun 3, 2020
@wojciechczerniak
Copy link
Contributor

Just a note to @AMBudnik gif. For future generations

As you can see the headers are kept in this example. It's because they are deliberately defined in the config and have custom names, therefore they should always be displayed. The columns and rows headers that were kept in v7 and are gone now, are the ones that are generated from the data set. Because the whole data set is removed, there are no rows, nor columns that can (or should) be shown

@aninde
Copy link
Contributor Author

aninde commented Jun 3, 2020

@scarletfog this may be useful to you for Release Notes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Nested rows Plugin Regression Issues that were created while adding new changes to the source code
Projects
None yet
Development

No branches or pull requests

5 participants