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] nestedHeaders and hiddenColumns don't work together #6879

Closed
pnowak opened this issue May 13, 2020 · 9 comments · Fixed by #7528
Closed

[8.0.0.-beta2-rev1] nestedHeaders and hiddenColumns don't work together #6879

pnowak opened this issue May 13, 2020 · 9 comments · Fixed by #7528
Assignees
Labels
bug Focus / blur Issues related to focus Hiding Plugin Nested headers Plugin Regression Issues that were created while adding new changes to the source code Status: Released
Milestone

Comments

@pnowak
Copy link
Contributor

pnowak commented May 13, 2020

Description

If we set both plugins, not all column headers are displayed.
Zrzut ekranu 2020-05-13 o 14 44 31

7.4.2 - https://jsfiddle.net/53dmngb7/1/
Zrzut ekranu 2020-05-13 o 14 44 48

8.0.0. beta-1 - https://jsfiddle.net/7a4obtck/
Zrzut ekranu 2020-05-13 o 14 44 39

Demo

https://jsfiddle.net/ep069nqy/1/

Your environment

  • Handsontable version: 8.0.0. beta-2-rev3
  • Browser Name and version: Chrome 80
  • Operating System: macOS Mojave
@AMBudnik
Copy link
Contributor

AMBudnik commented May 26, 2020

This was broken in 8.0.0-beta2-rev1 https://jsfiddle.net/AMBudnik/dcohnzvu/

Here's a second case to check.

Steps to reproduce

  1. Click Hide columns
  2. Scroll right to check the state of nestedHeaders.
  3. Click Show columns

Result
There are nestedHeaders missing when we hide columns with the button.
hide_bug_beta2rev1
8.0.0-beta2-rev1 - 8.0.0-beta2-rev8 hiding issue

after the selection of the C header.
Zrzut ekranu 2020-05-26 o 14 17 07

Demos

https://jsfiddle.net/AMBudnik/5c82p1xL/ 7.4.2 (works well)
https://jsfiddle.net/AMBudnik/0vfL8tm3/ (works well)
https://jsfiddle.net/AMBudnik/6cawp8n4/ 8.0.0-beta2-rev1 (broken)
https://jsfiddle.net/AMBudnik/xa7ckLyh/ 8.0.0-beta2-rev8 (broken)

Inform forum, and forum and Zen 25290, Gro 32275, Zen 26217, forum, and forum

@AMBudnik AMBudnik changed the title [8.0.0.-beta2-rev3] nestedHeaders and hiddenColumns don't work together [8.0.0.-beta2-rev1] nestedHeaders and hiddenColumns don't work together May 26, 2020
@aninde
Copy link
Contributor

aninde commented Jul 6, 2020

Still broken on latest rev 8.0.-beta2-rev18 https://jsfiddle.net/ep069nqy/1/

@AMBudnik
Copy link
Contributor

AMBudnik commented Aug 4, 2020

We should also test cases reported here #7160 after fixing this issue.

@AMBudnik
Copy link
Contributor

stable v8.0.0 https://jsfiddle.net/AMBudnik/z87h1v2g/ looks like the mentioned revision

Zrzut ekranu 2020-08-28 o 15 29 45

@wszymanski wszymanski removed their assignment Sep 9, 2020
@m4m4m4
Copy link

m4m4m4 commented Sep 23, 2020

Any ETA on this? Would love to upgrade to v.8 but this affects us heavily

@jansiegel jansiegel self-assigned this Sep 28, 2020
@AMBudnik
Copy link
Contributor

Hi @m4m4m4 the issue is currently in development

@budnix
Copy link
Member

budnix commented Oct 5, 2020

I add so that it does not disappear a PoC (unfinished) https://github.com/handsontable/handsontable/tree/feature/issue-6839

budnix added a commit that referenced this issue Mar 18, 2021
budnix added a commit that referenced this issue Mar 18, 2021
budnix added a commit that referenced this issue Mar 22, 2021
budnix added a commit that referenced this issue Mar 24, 2021
Move methods about creating IndexMaps from Core to IndexMapper.
Implement "mapNamesIgnoreList" option for index changes listener. Add
tests for NestedHeaders plugin cooperation with hiding columns.

Issue: #6879
budnix added a commit that referenced this issue Mar 25, 2021
budnix added a commit that referenced this issue Mar 25, 2021
budnix added a commit that referenced this issue Apr 1, 2021
The module provides the ability to observe the changes that happened in
the index map values during the code running. Currently, only the
"hiding" index map types can be observable. The commit covers all module
implementation with unit tests.

The "crossHidingColumns" meta property is moved from the tree data to
the source settings. Thanks to this, building a tree structure from
scratch (it happens after the "updateSettings" call) does not cause
metadata loss.

To the "hideColumn" and "showColumn" tree modifiers, there is added a
logic that ignores the action when the collapsible node is detected.
This allows postponing the bug fix related to collapsible headers to the
next phase.

Issue: #6879
budnix added a commit that referenced this issue Apr 2, 2021
Add more tests for the NestedHeaders plugin cooperation with hidden
columns (using hidden map). Add missing tests for new method added to
the NestedHeaders' state manager and add integration tests for the
ChangesObservable module cooperation with IndexMapper object.

Issue: #6879
budnix added a commit that referenced this issue Apr 6, 2021
budnix added a commit that referenced this issue Apr 12, 2021
That changes fixes the headers selection when the NestedHeaders plugin
is enabled. When the plugin is enabled it blocks the native columns
selection within the Selection module. The plugin is responsible for
column selection. Depends on what header level is clicked and how the
tree structure looks like the plugin based on its state selects the
correct columns.

To achieve that the new hook "beforeHighlightingRowHeader" is added. The
hook allows control the visual column index under the TH element will
be modified.

Issue: #6879
budnix added a commit that referenced this issue Apr 14, 2021
@budnix budnix linked a pull request Apr 15, 2021 that will close this issue
16 tasks
budnix added a commit that referenced this issue Apr 16, 2021
Issue: #6879
budnix added a commit that referenced this issue Apr 20, 2021
To decrease the overall memory footprint the index mapper instances are
nulled. Additionally, the redundant unit test is removed.

Issue: #6879
budnix added a commit that referenced this issue Apr 20, 2021
budnix added a commit that referenced this issue Apr 20, 2021
The change is compatible with how the build-in selection works. When
no table header is selected it's possible to select the header using
RMB. Otherwise, it's not possible.

Issue: #6879
budnix added a commit that referenced this issue Apr 26, 2021
The commit fixes an issue where hiding columns causes nested headers to glitch.
Several modules had to be extended with new functionalities to get the state
where nested headers work correctly.

 * New modifiers are added to the nodeModifier module. The nested headers tree
structure can be manipulated by the new "hideColumn" and "showColumn"
modifiers. Within the PR, the modifiers are split into separate files for
better maintainability.
 * The "matrixGenerator" module is rewritten to support new tree metadata props
(crossHiddenColumns).
 * For the IndexMapper module, the "changesObservable" module is added. The
module allows observing the index changes changed during HoT runtime.
Currently, only the "hiding" map is supported. Thanks to that the tree does not
have to build from scratch but can be modified on index change.
 * To the IndexMapper, the new methods are added. The methods complement the
IndexMapper API. So from the Handsontable instance, there is a possibility to
create index maps and use them to manipulate rows/columns behavior. For
instance: `hot.rowIndexMapper.createAndRegisterIndexMap`.
 * The header highlighting process is fixed. From, now on only the headers
closest to the dataset are highlighted (grey). An only active highlight (blue)
is applied to all header layers.

It's worth noticing that the PR is focused on fixing the cooperation of the
NestedHeaders plugin and the HidingColumns (and hiding map in general). The
collapsible headers will be fixed in separate PR. Moreover, the PR changes how
the selection of the nested headers is held. Previously no matter what header
level was clicked the selection coordinates started from -1.

The list of backward-compatible changes:
 * New `createAndRegisterIndexMap()` method for IndexMapper;
 * New `createChangesObserver()` method for IndexMapper;
 * New `unregisterAll()` method for IndexMapper;
 * New `beforeHighlightingColumnHeader` and `beforeHighlightingRowHeader` hooks.

Issue: #6879
@aninde
Copy link
Contributor

aninde commented May 5, 2021

This issue will be fixed by the v.8.4.0
Screenshot 2021-05-05 at 15 10 05

Demo: https://jsfiddle.net/t01naufr/

@AMBudnik
Copy link
Contributor

AMBudnik commented May 11, 2021

@aninde so much better! That's a relief to see this issue fixed.

@m4m4m4 I'm sure you'll benefit from this fix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Focus / blur Issues related to focus Hiding Plugin Nested headers Plugin Regression Issues that were created while adding new changes to the source code Status: Released
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants