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

Wrong typing for beforeColumnMove #5416

Closed
guillaumep opened this issue Sep 25, 2018 · 8 comments
Closed

Wrong typing for beforeColumnMove #5416

guillaumep opened this issue Sep 25, 2018 · 8 comments

Comments

@guillaumep
Copy link
Contributor

guillaumep commented Sep 25, 2018

Documentation says about beforeColumnMove's arguments:

Name Type Description
columns Array of numbers Array of visual column indexes to be moved.
target Number Visual column index being a target for moved columns.

https://docs.handsontable.com/pro/5.0.2/Hooks.html#event:beforeColumnMove

But the typing in handsontable.d.ts has:

beforeColumnMove?: (startColumn: number, endColumn: number) => void;

It should be:

beforeColumnMove?: (columns: number[], target: number) => void;

@guillaumep
Copy link
Contributor Author

Note: same thing is happening for beforeRowMove.

@AMBudnik
Copy link
Contributor

Thank you for sharing @guillaumep

@guillaumep
Copy link
Contributor Author

@wojciechczerniak @AMBudnik I approve of task #5094, where the issue I have logged should eventually be addressed, but can I suggest this current PR be merged in the sort term, so I and other Handsontable user can benefit from correct typings sooner?

@AMBudnik
Copy link
Contributor

AMBudnik commented Oct 4, 2018

Hi @guillaumep

we haven't scheduled the epic nor the PR yet but will surely consider adding the PR first

@guillaumep
Copy link
Contributor Author

guillaumep commented Oct 22, 2018

@AMBudnik Is there something I can do to help this get merged?

It has already been one month the PR has been opened. Do other contributors to Handsontable have to wait this long too?

@AMBudnik
Copy link
Contributor

I will try to push the code review for #5425 to the next milestone

@guillaumep
Copy link
Contributor Author

@AMBudnik Thanks!

@AMBudnik
Copy link
Contributor

Hi @guillaumep

the changes are published. Please update to 6.2.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants