Skip to content

Fix #1685: Remove col.index from most code #1692

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

Closed
wants to merge 1 commit into from

Conversation

PaulL1
Copy link
Contributor

@PaulL1 PaulL1 commented Oct 3, 2014

Removed col.index everywhere it appears, except for in the cell content
templates, which appear to use col.index in creating the cell class.

Added a column renumber to buildColumns to always give every column an
index starting at zero.

Reviewed every tutorial to see if there was breakage. Found breakage, but only one item that related to this change (the rest was pre-existing).

One outstanding defect that I couldn't resolve (nor see how this change caused it, although it clearly did) is that tutorial 113 - adding and removing columns - doesn't correctly splice a new column in the middle. The data turns up, but the header widths appear to be slightly off, and therefore everything is wrapping. Upon scrolling the data fixes itself, the headers don't.

Looking for a review and suggestions on what could be causing that, rather than expecting this to be merged.

Removed col.index everywhere it appears, except for in the cell content
templates, which appear to use col.index in creating the cell class.

Added a column renumber to buildColumns to always give every column an
index starting at zero.
@PaulL1
Copy link
Contributor Author

PaulL1 commented Oct 4, 2014

Replaced by #1705

@PaulL1 PaulL1 closed this Oct 4, 2014
@c0bra c0bra removed the in progress label Oct 4, 2014
@PaulL1 PaulL1 deleted the 1685_first_column_width branch October 4, 2014 00:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants