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-rev1] Data from a merged cell is gone if its part is hidden - empty editor #6872

Closed
AMBudnik opened this issue May 13, 2020 · 13 comments
Assignees
Labels
bug Hiding Plugin Merge cells Plugin Regression Issues that were created while adding new changes to the source code

Comments

@AMBudnik
Copy link
Contributor

AMBudnik commented May 13, 2020

Description

Data from a merged cell is gone if its part is hidden. It is a regression from this issue #6871 - previously it was just a UI issue, in 8.0.0.-beta2 data is also gone from the editor.

Steps to reproduce

  1. Go to https://jsfiddle.net/xc7uaohL/
  2. Open editor for the merged cell - tthere should be B2 in the editor but it's blank
  3. Type test in the merged cell - you can see test in the cell
  4. Click the button below to log data in the console
  5. Show columns
  6. Unmerge cells - the old data is visible - B2, not test
  7. Click the button below to log data in the console - you've changed C2, not B2

If you at this point click the button, you'll get B2 on its position and test on C2. Which also reflects if you unmerge the cells.

beta2

Previously

  1. Go to https://jsfiddle.net/xc7uaohL/
  2. Open editor for the merged cell - there's B2
  3. Type test in the merged cell - you cannot see test in the cell
  4. Click the button below to log data in the console
  5. Show columns
  6. Unmerge cells - the NEW data is visible - test
  7. Click the button below to log data in the console - you've changed B2

beta1

Demo

https://jsfiddle.net/handsoncode/8ffpsqt6/ 7.4.2
https://jsfiddle.net/4brxm2df/2/ 8.0.0.-beta2 from commit 929b22c (last working)
https://jsfiddle.net/4brxm2df/3/ 8.0.0.-beta2 from commit 81020af (first broken)
https://jsfiddle.net/4brxm2df/4/ 8.0.0.-beta-rev4 (still broken)

Related issues

Your environment

  • Handsontable version: 8.0.0.-beta2 from commit 81020af
  • Browser Name and version: Chrome 80
  • Operating System: macOS Catalina
@AMBudnik AMBudnik added bug Merge cells Plugin Hiding Plugin Regression Issues that were created while adding new changes to the source code labels May 13, 2020
@AMBudnik AMBudnik changed the title [8.0.0.-beta2-rev1] Data from a merged cell is gone if it's the first part is hidden - empty editor [8.0.0.-beta2-rev1] Data from a merged cell is gone if its part is hidden - empty editor May 13, 2020
@AMBudnik
Copy link
Contributor Author

AMBudnik commented May 13, 2020

If we go to https://jsfiddle.net/xc7uaohL/ and click CTRL/CMD + Z to undo the C2 is back.
Zrzut ekranu 2020-05-13 o 14 39 26

@wszymanski
Copy link
Contributor

wszymanski commented May 26, 2020

May be related to #6873.

@aninde

This comment has been minimized.

@AMBudnik
Copy link
Contributor Author

AMBudnik commented Jun 4, 2020

Case 2

Steps

  1. Go to https://jsfiddle.net/AMBudnik/kz98cq1e/ 8.0.0-beta2-rev3
  2. Select A1
  3. Hold CMD/CTRL and click E2

Result
You selected A1:E3 instead of A1:E2.

issue

The issue's also replicable in 8.0.0-beta2-rev10 https://jsfiddle.net/AMBudnik/k6robep7/1/

@wojciechczerniak
Copy link
Contributor

Case 2

Steps

  1. Go to https://jsfiddle.net/AMBudnik/kz98cq1e/ 8.0.0-beta2-rev3
  2. Select A1
  3. Hold CMD/CTRL and click E2

Result
You selected A1:E3 instead of A1:E2.

issue

The issue's also replicable in 8.0.0-beta2-rev10 https://jsfiddle.net/AMBudnik/k6robep7/1/

Exactly how it supposed to. This is the desired behavior. Check XL and GS.

@AMBudnik
Copy link
Contributor Author

AMBudnik commented Jun 4, 2020

@wojciechczerniak is right. Works the same way in Google Sheets 🤔 but I did not change my mind when it comes to UX this looks odd. Nevertheless, we can check if it works the same way after we get some fresh new commits 👍

@AMBudnik
Copy link
Contributor Author

AMBudnik commented Jun 4, 2020

@wojciechczerniak what about this, let's call it case 3

Case moved to #6978

Steps
1. Select both columns
2. Use Merge cells from the context menu
3. Use CTRL/CMD + Z to undo

Result
merge-show-undo

I would expect to get the same result as we get (cause we do not support undo-redo for hiding/showing) but with header A highlighted (not all of them).

Here's Google Sheets reproduction for comparison

gs

@wojciechczerniak
Copy link
Contributor

True. Only A header should be selected. Is this a regression?

For undo/redo hidding column we should have a separate issue. This is out of scope

@AMBudnik
Copy link
Contributor Author

AMBudnik commented Jun 4, 2020

True. Only A header should be selected. Is this a regression?

Broken in 8.0.0-beta2-rev10 https://jsfiddle.net/AMBudnik/2t5sdgoz/ as well as 7.4.2 (and 6.2.2) https://jsfiddle.net/AMBudnik/bp0xrv1y/ - not a regression. I'll move it to a separate issue.

@wszymanski
Copy link
Contributor

When we merge cells, the first of which is hidden, we don't see the content and we have an empty, merged cell. http://jsfiddle.net/aninde/ehs1obzq/1 8.0.0-beta2-rev10. When the first row ceases to be hidden A2 is displayed on the merged cells.

Probably related to the Handsontable 7+. Please check if extra information should be added to the #6871.

@AMBudnik
Copy link
Contributor Author

AMBudnik commented Jun 4, 2020

@wszymanski @aninde

Probably related to the Handsontable 7+. Please check if extra information should be added to the #6871.

all - 8.0.0.beta2-rev6 http://jsfiddle.net/AMBudnik/mvntg5pb/
6 2 2

8.0.0.beta2-rev7 - rev10 http://jsfiddle.net/AMBudnik/2yq6zj91/
Zrzut ekranu 2020-06-4 o 11 44 00

We may say that it never displayed the value and before rev7 it was broken even worse.

EDIT: Works the same for rev11 http://jsfiddle.net/AMBudnik/43poemx6/

@aninde
Copy link
Contributor

aninde commented Jun 4, 2020

@AMBudnik you're right, this issue (#6872 (comment)) has already been partially reported here #6224 (comment).

@AMBudnik
Copy link
Contributor Author

AMBudnik commented Jun 4, 2020

The main issue is solved in 8.0.0-beta2-rev11 https://jsfiddle.net/AMBudnik/ctjm0zhv/ it works the same way as it worked before the regression.

As case 2 is intended, case 3 is moved and the case with

When we merge cells, the first of which is hidden, we don't see the content

is reported in other issues we can close this topic.

Thanks guys!

@AMBudnik AMBudnik closed this as completed Jun 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Hiding Plugin Merge cells Plugin Regression Issues that were created while adding new changes to the source code
Projects
None yet
Development

No branches or pull requests

4 participants