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

Changes in colToProp and propToCol methods #7144

Draft
wants to merge 25 commits into
base: develop
Choose a base branch
from

Conversation

wszymanski
Copy link
Contributor

@wszymanski wszymanski commented Jul 21, 2020

Context

This PR realise idea to return null for indexes beyond the table boundaries while calling propToCol method. However, there should be done yet more significant changes which affect lot of places in Handsontable.

  • propToCol returns null for not existing properties
  • colToProp returns null for indexes beyond the table boundaries
  • colToProp return null for not existing property

// TODO: This should be removed. Please keep in mind that the `getSourceDataAtCol` and `getSourceDataAtCell` use it.
if (Number.isInteger(column) === false) {
return column;
}

  • should colToProp return -1 after handling -1 as argument (considering row header)?
    • colToProp return null after handling -2 as argument (considering row header)?
  • should colToProp and propToCol work on physical indexes (to perform translation also on trimmed indexes properly)?
    • should colToProp(number) and propToCol(number) return number or translated number (to visual/physical index)?
  • should colToProp(x) === propToCol(colToProp(x))?
  • methods such as getSourceDataAtCol, getSourceDataAtCell and getAtCell (internally function) work on physical indexes.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jul 21, 2020

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit f6dac72:

Sandbox Source
vanilla-handsontable-pr Configuration

src/dataMap.js Outdated Show resolved Hide resolved
@wszymanski wszymanski linked an issue Sep 22, 2021 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Methods colToProp and propToCol should work properly
4 participants