handsontable
handsontable copied to clipboard
Fix an issue with dropdownMenu not being displayed on a nestedHeaders-enabled instance with all rows trimmed out.
Context
The bug described in #9753 was caused by a more broad issue:
When the Handsontable instance does a full redraw from a callback run on afterOnCellMouseDown
, all the callbacks for the succeeding click
event bound to the rootElement
are blocked by the fact that the DOM content of rootElement
's children was modified.
- In the simplest example: https://jsfiddle.net/js_ziggle/ecqg5w9b/
- In Handsontable context, this can be achieved without
nestedHeaders
,filters
, anddropdownMenu
: https://jsfiddle.net/js_ziggle/1mxgrtco/ By passing a custom header-rendering function, we're forcing the full redraw of those. Clicking on them won't trigger theclick
event callback defined below.
My fix for #9753 pushes the render-triggering function (selecting columns) after all the previously-declared click
callbacks were already run by placing it at the end of the click
event stack and asynchronously removing the newly-added callback.
Additionally, I corrected some typos in the code I found while searching for this fix.
How has this been tested?
Added a test case for the issue.
Types of changes
- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature or improvement (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [ ] Additional language file or change to the existing one (translations)
Related issue(s):
- #9753
Affected project(s):
- [x]
handsontable
- [ ]
@handsontable/angular
- [ ]
@handsontable/react
- [ ]
@handsontable/vue
- [ ]
@handsontable/vue3
Checklist:
- [x] I have reviewed the guidelines about Contributing to Handsontable and I confirm that my code follows the code style of this project.
- [x] I have signed the Contributor License Agreement
- [ ] My change requires a change to the documentation.
Launch the local version of documentation by running:
npm run docs:review 4ec2da9a3875ac37395c46d947cd94931a965715
@budnix In https://github.com/handsontable/handsontable/pull/9846/commits/c7bb49f24378def1ea364f750632df445f679bc0 I took your idea from https://github.com/handsontable/handsontable/commit/08895d41f91294e5de8521b05423c951a0f8cf8d, but modified it to utilize both calculators for fully visible rows/columns and partial ones. It looks like it works fine, but I'm not 100% sure if it doesn't break your initial idea that introduced this issue. Could you verify if it makes sense?
Does it also fix #9930?
Does it also fix #9930?
Might be, I'll check that
@budnix In c7bb49f I took your idea from 08895d4, but modified it to utilize both calculators for fully visible rows/columns and partial ones. It looks like it works fine, but I'm not 100% sure if it doesn't break your initial idea that introduced this issue. Could you verify if it makes sense?
Initially, the rendering clue seemed like what needed to be fixed. But after playing with that, I saw some issues. The first problem is performance. On every scroll move, there are two more calculators created only for the count
value check - that hurts a little bit. And second, for oversized rows, the issue still exists - the dropdown menu can't be opened.
But it seems that a few days between code reviews refreshed my mind 😄 I reinvested the issue and found that the column (and row) for non-nested headers had the same issue. They were fixed 7 years ago by adding DOM caching layer - that was a part of the performance task (https://github.com/handsontable/handsontable/commit/fbfcbb44525461ac83b68c3f27ebbfd17522911b). The NestedHeaders plugin lacks that cache layer implementation that's why I'd propose adding one. I've created a new PR with the proposition. The new PR contains all the relevant changes from this one.