Handsontable: beforeColumnResize and beforeRowResize first argument is column width

[kakabomba]

in beforeColumnResize hook first passed argument is column width not column index
beforeColumnResize: function (currentColumn, newSize, isDoubleClick) {
console.log(currentColumn, newSize, isDoubleClick);
},

gives
154, 154, undefined

[AMBudnik]

Confirmed
Demo: http://jsfiddle.net/handsoncode/3gpe2qq5/

[krzysztofspilka]

Not sure about the severity, @jansiegel could you confirm the priority of this issue?

[cgatesman]

I ran into this, too. currentColumn is blown away by the return value of the previous hook call to AutoColumnSize. With no column index context, beforeColumnResize hook is pretty much unusable.

[IntegerMan]

Yep. This is definitely a thing. We were actually getting other exceptions during certain column resizes and trying to reduce the surface area of those bugs by disabling column resize (we didn't want the columns to be resized anyway). This really didn't help very much at all.

[gtiguy92]

+1 onBeforeColumnResize returns the old size which is then sent as the first parameter to the whatever you have hooked to beforeColumnResize. We are expecting the first parameter to be currentColumn. Support for oldSize and newSize, along with currentColumn, would be awesome, but without currentColumn it is very hard to use this hook.

[gtiguy92]

I was wrong in my statement above, here's exactly what's going on.

Handsontable Docs:
beforeColumnResize(currentColumn, newSize, isDoubleClick)

Actual Resize Larger:
beforeColumnResize(newSize, newSize, isDoubleClick)

Actual Resize Smaller:
beforeColumnResize(newSize, newSize, isDoubleClick)

Actual Resize via Double Click:
beforeColumnResize(newSize, oldSize, isDoubleClick)

None of the actuals match the documentation and the actuals are not consistent. This really makes this hook unusable.

[IntegerMan]

I did look into the HoT JavaScript upon encountering this and it did look to at least start the invoke correctly (though it often passed undefined for isDoubleClick), but for whatever reason, that's not the value that is received by the consumer, particularly for the currentColumn property. I'm really not sure what's going on there, but I doubt it's what anyone intended.

[cgatesman]

See my previous comment to what is happening in the HOT code. There is a chain of hooks going on. The return of the previous hook is the first parameter to the next hook. The problem is the previous hook to onBeforeColumnResize is returning the newSize, so it blows away the expected currentColumn first parameter to onBeforeColumnResize.

The actual problem could be with the order of these hooks not being correct, AutoColumnSize (previous hook) not returning the correct value for onBeforeColumnResize, or the way the hook handler is implemented is not right passing the return of a hook as the first parameter to the next hook.

[wszymanski]

Actually function run(context, key, p1, p2, p3, p4, p5, p6) of Hooks class (responsible for handling the stack of hooks) work as described below.

for n < |arrayOfHooks|
  hookCallResult = callHook(n)
  if hookCallResult
    changeFirstParametrOfNextHookCall

In other words, hooks are called in a loop and if any of them return something we pass it as a first parameter of next function call (parameter p1 is changed). Thus parameter p1 is used to pass the result of hook execution.

We should rewrite run function to handle results of previous function calls in a proper way. Proposition how to rewrite execution of internal call functions is given below.

Change from:

call(context, p1, p2, p3, p4, p5, p6) 

to:

call(context, resultOfPreviousCall, p1, p2, p3, p4, p5, p6) 

Problem described in ticket occurs because AutoColumnSize plugin always add beforeColumnResize hook (in constructor) and corresponding onBeforeColumnResize function returns value.

[AMBudnik]

[G #4513]

[sakalys]

Still unresolved I presume. @wszymanski you are absolutely right. Will any decision maker make the choice whether to deal with the issue that

[sonmunehiro]

When will it be fixed?

[AMBudnik]

I do not see it on our roadmap for this month. Our developer @wszymanski have already done some research on this issue and found (as mentioned above) that this fix requires a greater change in the code, which we can't afford now to perform.

[AMBudnik]

Just to confirm. This issue is still replicable in v 2.0.0 http://jsfiddle.net/qaf3wqut/

[kakabomba]

I missed the second anniversary!! :laughing:

[AMBudnik]

OMG, you're right @kakabomba
So many updates and the issue is still alive

[MalloryEaton]

Any news on when this is going to be fixed?

[AMBudnik]

I hope that we'll be able to fix the issue with https://github.com/handsontable/handsontable/issues/5112 but this is a very complex issue and we've did not schedule it yet

[craigbenstock]

Hi, I have a use case where I need certain columns to be fixed width and others to allow manualColumnResize.

As a fix for this issue seems nowhere in sight could you please help me with a workaround?

I build my columns off of an internal array (myColumns) which I can reference the 'currentColumn' index against from 'afterColumnResize'. Those that have an 'isFake' bool set to true are the ones I wish to prevent resize on.

I was hoping the below bit of code inside 'afterColumnResize' would set them back to 90 width but instead it does nothing.

"afterColumnResize": function(currentColumn, newSize, isDoubleClick) {
var column = myColumns[currentColumn];
if (column.isFake) {
return this.getPlugin("manualColumnResize").setManualSize(currentColumn, 90);
}
},

Can you help please?

[craigbenstock]

I'll leave it there in case it's useful, but please ignore my last comment. I just needed to call this.render(); after setManualSize.

[wojciechczerniak]

We should change the order of arguments. newSize should be the first argument, and selectedCol second one. This way calbacks can change newSize as exptected and column number will never change.

• [ ] Change the order of newSize and selectedCol arguments in both runHooks calls
• [ ] Double click should have third argument as false not undefined

Unfortunately I have to mark this as a breaking change and we have to wait for the next major version.

[pnowak]

@wojciechczerniak I have a question about afterColumnResize and afterRowResize: to be consistency after these changes maybe we should change order too and rename currentColumn to selectedCol? And maybe we should use selectedCololumn not shortened selectedCol?

[wojciechczerniak]

@pnowak I think it should be simply row and column. Without "current" or "selected" prefixes. This hook might not always be called on selection.

And yes, we should use full column instead of shortened col.

[AMBudnik]

I'm writing just to confirm that the new order

afterColumnResize?: (newSize: number, column: number, isDoubleClick: boolean)
afterRowResize?: (newSize: number, row: number, isDoubleClick: boolean)

Works well for 8.0.0-beta2-rev16 https://jsfiddle.net/b52g1n48/

[pnowak]

Hi @kakabomba @IntegerMan @cgatesman @gtiguy92 @sakalys @sonmunehiro @MalloryEaton @craigbenstock

I have good news! We've just released 8.0.0-beta2 that fixes the mentioned issue. We won't be closing this ticket now. It will be closed after we release the final 8.0.0 version of Handsontable.

Here’s a full list of changes https://github.com/handsontable/handsontable/releases/tag/8.0.0-beta.2
NPM https://www.npmjs.com/package/handsontable/v/8.0.0-beta.2

All feedback's appreciated. Thank you for your input.

[pnowak]

Works well for 8.0.0-beta2 - https://jsfiddle.net/La5kdj04/

[AMBudnik]

Hi @kakabomba @craigbenstock @MalloryEaton @cgatesman @IntegerMan @sakalys @gtiguy92

I’m glad to announce that Handsontable v8.0.0 that solves this issue is live! 


Updated demo https://jsfiddle.net/s0wgb5fh/

CDN https://cdn.jsdelivr.net/gh/handsontable/handsontable@latest/dist/handsontable.full.css
https://cdn.jsdelivr.net/gh/handsontable/handsontable@latest/dist/handsontable.full.js
NPM npm i handsontable
Release Notes https://github.com/handsontable/handsontable/releases



I highly encourage you to test the new version and leave some feedback. I hope that the amount of work that we put on the v8 will help you to improve your project.