-
Notifications
You must be signed in to change notification settings - Fork 222
- add colSpan prop to Td objects #135
base: master
Are you sure you want to change the base?
Conversation
paulcameron
commented
Apr 7, 2015
- colSpan prop for Td objects
- assumes that null data for remaining cols should not be rendered
(#84) |
How does this work with filtering and sorting? I'd like to see some test cases inside your block that cover those |
Besides that caveat this is very nice. |
i'm planning on working on a separate PR that addresses sorting. i want to add the ability to specify XX number of rows that get pinned to the bottom of the table. but yes, sorting "works" however the results can be oddly defined if you are sorting on a col that has no corresponding Td. i'll look at how it deals with filtering. this is n't part of our use case, but i'll make sure that it isn't too janky |
👍 sounds good. I'm totally fine with accepting a partial PR. |
|
ok, that fixes the sorting issues. |
@glittershark all good when you get a chance |
@paulcameron day job has been demanding more and more of my attention lately 😄 should be able to take a look at this over the weekend. |
That's great. I was just making sure you saw my update. On Wednesday, April 29, 2015, Griffin Smith [email protected]
|
@@ -311,9 +313,21 @@ | |||
value = value.value; | |||
} | |||
|
|||
if(typeof(props.colSpan) !== 'undefined') { | |||
colSpanDebt += props.colSpan - 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we fail with a warning if a user specifies a colSpan of less than 1 here?
@paulcameron do you have the time to rebase this? If not I'll see if I can stumble my way through it |
Any updates on this PR? |
So what's the status on adding colspan? |
@parthms @sthadeshwar i have moved on to other things. you guys can feel free to pick it up from here. |