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] We cannot add a column on the left when there's no data (data is filtered out) #6998

Closed
AMBudnik opened this issue Jun 8, 2020 · 2 comments
Assignees
Labels
bug Filtering Plugin Regression Issues that were created while adding new changes to the source code Status: Merged (ready for release)
Milestone

Comments

@AMBudnik
Copy link
Contributor

AMBudnik commented Jun 8, 2020

Description

We cannot add a column on the left when there's no data (data is filtered out)

Steps to reproduce

  1. Filter out all the rows
  2. Open dropdown menu

Result
You cannot add a column on the left, but can add one on the right.
Zrzut ekranu 2020-06-8 o 15 00 18

Expected result
Zrzut ekranu 2020-06-8 o 15 01 15

Demo

https://jsfiddle.net/AMBudnik/yo7psbwz/ 7.4.2 (works well)
https://jsfiddle.net/AMBudnik/mr7ej1zp/ 8.0.0-beta1 (wroks well)
https://jsfiddle.net/AMBudnik/dr32xbLs/ 8.0.0-beta2-rev1 (broken)
https://jsfiddle.net/AMBudnik/o26w0dhf/ 8.0.0-beta2-rev12 (still broken)

Additional info

Your environment

  • Handsontable version: 8.0.0-beta2-rev1
  • Browser Name and version: Chrome 80
  • Operating System: macOS Catalina
@AMBudnik AMBudnik added bug Filtering Plugin Regression Issues that were created while adding new changes to the source code labels Jun 8, 2020
@wszymanski wszymanski removed their assignment Jun 9, 2020
@budnix budnix self-assigned this Jun 16, 2020
budnix added a commit that referenced this issue Jun 18, 2020
The header selection is restored by passing through the negative values
through the whole library. This braking change forced all tests to be
adjusted.

Issue: #6998
@wojciechczerniak wojciechczerniak added this to the June 2020 milestone Jun 19, 2020
budnix added a commit that referenced this issue Jun 19, 2020
In this commit I separate how the coordinates are used within
Walkontable and Handsontable context. The Walkontable renderer needs to
know about negative coordinates as that module render the whole table.
In The Handsontable context, however only coordinates for cells are
important mostly. As in that context exists cell meta objects which
exists only for cells.

Issue: #6998
budnix added a commit that referenced this issue Jun 19, 2020
Change methods in the same manner as was done for "getTopLeftCorner",
"getOuterTopLeftCorner" and similar methods.

Issue: #6998
budnix added a commit that referenced this issue Jun 19, 2020
budnix added a commit that referenced this issue Jun 19, 2020
While fixing the negative coordinates I've spotted that alignment and
comments command from context menu doesn't work when the selection was
made in different direction that top-left to bottom-right.

Issue: #6998, #6600
budnix added a commit that referenced this issue Jun 19, 2020
Adjust the logic how the selection was processed by the plugins. I
removed all references to negative indexes.

Issue: #6998
budnix added a commit that referenced this issue Jun 22, 2020
Add condition for retrieving only cell coordinates (ignoring header)

Issue: #6998
@aninde
Copy link
Contributor

aninde commented Jun 22, 2020

This issue is fixed on merge-candidate PR#7027 for 8.0.0-beta2
https://jsfiddle.net/nmh35vLc/ vPR#7027
fixed-PR#7027

budnix added a commit that referenced this issue Jun 22, 2020
The indexes equal to 0 are wrongly excluded from the borders logic.

Issue: #6998
budnix added a commit that referenced this issue Jun 24, 2020
When the header was selected and merged cells or alignment
operations were undoing, an exception was thrown. This was caused
by negative coordinates. I fixed this by excluding negative
coordinates from the process.

Issue: #6998
budnix added a commit that referenced this issue Jun 24, 2020
Split long line.

Issue: #6998
budnix added a commit that referenced this issue Jun 24, 2020
budnix added a commit that referenced this issue Jun 24, 2020
When the row or column header was selected the AutoFill handler was
unresponsive on filling action. This is caused by negative coordinates.
I fixed this by coords normalization.

Issue: #6998
budnix added a commit that referenced this issue Jun 24, 2020
budnix added a commit that referenced this issue Jun 24, 2020
budnix added a commit that referenced this issue Jun 25, 2020
When row or column header was selected and enter was hit then the editor
position was out of sync by 1px relative to the cell. I've corrected the
condition in the editors code which includes 1px only for row and column
equals to 0.

Issue: #6998
budnix added a commit that referenced this issue Jun 25, 2020
Consistent with the counterpart file (showRow.js)

Issue: #6998
budnix added a commit that referenced this issue Jun 26, 2020
budnix added a commit that referenced this issue Jun 30, 2020
The PR restores headers' selection when the indexes are trimmed. 
The header selection is necessary for context menu or dropdown menu
actions, which depend on coordinates. Moreover, for better user 
experience, it's better to make headers responsive.

I fix the issue by forwarding the coordinates object without changing
the negative coordinates. Thanks to that change I was able to get rid 
of all workarounds (1, 2, 3) caused by index mapper and made for 
selection.

The changes change the selection module in how it processes the 
coordinates. The CellRange class from now can contain negative 
values, except highlight property, which as the documentation says
points to cells only. That's why the coordinates for highlighting are
normalized to 0 when negative values are passed.

Most of the context menu options were normalized about its
disability state.

Issue: #6998

Co-authored-by: Wojciech Szymański <141330+wszymanski@users.noreply.github.com>
budnix added a commit that referenced this issue Jun 30, 2020
* Revert headers selection when all indexes are trimmed

The header selection is restored by passing through the negative values
through the whole library. This braking change forced all tests to be
adjusted.

Issue: #6998

* Separate coordinates context between WoT and HoT

In this commit I separate how the coordinates are used within
Walkontable and Handsontable context. The Walkontable renderer needs to
know about negative coordinates as that module render the whole table.
In The Handsontable context, however only coordinates for cells are
important mostly. As in that context exists cell meta objects which
exists only for cells.

Issue: #6998

* Change getHight and getWidth methods

Change methods in the same manner as was done for "getTopLeftCorner",
"getOuterTopLeftCorner" and similar methods.

Issue: #6998

* Fix getHight, getWidth for selection

Issue: #6998

* Fix alignment and comments ops from context menu

While fixing the negative coordinates I've spotted that alignment and
comments command from context menu doesn't work when the selection was
made in different direction that top-left to bottom-right.

Issue: #6998, #6600

* Fix issues revealed after changing selection methods

Adjust the logic how the selection was processed by the plugins. I
removed all references to negative indexes.

Issue: #6998

* Fix Border context menu command

Add condition for retrieving only cell coordinates (ignoring header)

Issue: #6998

* Fix wrong assumption

The indexes equal to 0 are wrongly excluded from the borders logic.

Issue: #6998

* Fix undoing merge cells and alignment operations

When the header was selected and merged cells or alignment
operations were undoing, an exception was thrown. This was caused
by negative coordinates. I fixed this by excluding negative
coordinates from the process.

Issue: #6998

* Fix eslint complains

Split long line.

Issue: #6998

* Add tests for merge cells and alignment ops

Issue: #6998

* Fix unresponsive AutoFill handler

When the row or column header was selected the AutoFill handler was
unresponsive on filling action. This is caused by negative coordinates.
I fixed this by coords normalization.

Issue: #6998

* Fix selection after autofill operation

Issue: #6998

* Fix eslint complains

Issue: #6998

* Fix editor positioning

When row or column header was selected and enter was hit then the editor
position was out of sync by 1px relative to the cell. I've corrected the
condition in the editors code which includes 1px only for row and column
equals to 0.

Issue: #6998

* Make the logic for showColumn more consistent

Consistent with the counterpart file (showRow.js)

Issue: #6998

* Update src/editors/textEditor.js

Co-authored-by: Wojciech Szymański <141330+wszymanski@users.noreply.github.com>

* Update src/plugins/comments/test/comments.e2e.js

Co-authored-by: Wojciech Szymański <141330+wszymanski@users.noreply.github.com>

* Add ability to clone CellRange instance

Issue: #6998

* Fix unselectable headers when all indexes are hidden

The header selection generally contains cell selection coordinates. In a
case when all rows (or columns) are hidden that visual coordinates are
translated to renderable coordinates that do not exist. Hence no header
highlight is generated. In that case, to make a column (or a row) header
highlight, the row and column index has point to the header (the
negative value).

Issue: #7051

* Add ability to highlight headers for hidden indexes

Add ability to highlight headers (gray color) when one of the indexes
(row or column) are hidden and the second index is visible.

To support header highlight for that cases the `getCorner` method of the
Selection class is overwritten. There are two approaches to convince the
header Selection to highlight specific headers. One overwrite the method
which controls coordinates for headers (chosen approach for this commit)
and second rewrite/adjust CellCoords, CellRange, and correct all methods
that deal with numbers to support `null` values - to match changes.

Issue: #7051

* Improve code readability

Issue: #7051

* Change method name

Issue: #7051

Co-authored-by: Wojciech Szymański <141330+wszymanski@users.noreply.github.com>

Co-authored-by: Wojciech Szymański <141330+wszymanski@users.noreply.github.com>
@AMBudnik
Copy link
Contributor Author

Issue solved in v 8.0.0-beta2-rev18 https://jsfiddle.net/b7wveznq/

solved

You may see that the filters are attached to the wrong column, however, that's a separate issue (not a regression) reported here #6830

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Filtering Plugin Regression Issues that were created while adding new changes to the source code Status: Merged (ready for release)
Projects
None yet
Development

No branches or pull requests

5 participants