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

Fix to not always expect return string in _fnGetCellData to insert in innerHTML; and permit to call a callback #198

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

amnambiar
Copy link

@amnambiar amnambiar commented Jan 23, 2022

Hi Team,

IMPORTANT

Please test and accept this PR which will faciliate the following:

col.renderReturnType = 'function'; // only then will these changes be activated
const createdCellCallback = (tdElm, cellData, rowData, rowIndex) => {
  // dynamically insert an angular component into tdElm with help of Renderer2; passing in needed values from rowData and cellData as @Inputs to the component
}

col.render = (data, type, row, meta) => { return createdCellCallback; }

col.createdCell = createCellCallback;

This will faciliate blindly performing the function within returned function of col.render at all times (especially after reorder of columns)

This is in accordance to my communication over the issue thread DataTables/ColReorder#49

Cc: @AllanJard, @SandyDatatables @colin0117 Please look into this. I would like this feature included in matter of hours to be exact, as my app relies on this! Please HELP

@amnambiar
Copy link
Author

Can you please merge this PR in immediate effect in a matter of less than 24 hours, considering this a priority. PLEASEEE!

@hhariharan94
Copy link

+1

1 similar comment
@AnuShankar93
Copy link

+1

@AllanJard
Copy link
Contributor

Many thanks for the PR. However, I'm afraid I won't be able to pull it in yet for the following reasons:

  1. No example showing the changes in use
  2. No documentation updates relating to the changes (it looks like there is a new renderReturnType parameter which isn't defined anywhere
  3. I don't actually fully understand the change. Why can't you just pass the extra information into the cellData function rather than using a switch to call it with a different signature?
  4. I'd like to see an example showing how you handle orthogonal data types such as sorting data as well as a simple example. It isn't clear to me how that is handled with these changes.

I would like this feature included in matter of hours to be exact, as my app relies on this!

I'm afraid it doesn't work like that. I don't place demands on your time - I would appreciate if you didn't make demands of ours.

@amnambiar
Copy link
Author

@AllanJard ,

http://live.datatables.net/qonidogo/6/ here is the link to reproduce the issue.

Reorder the columns and see that the links in Name column disapper.

I want the table to behave in a way that if , commented lines are uncommented in place of existing 'render', then links be inserted no matter what.

Feel free to suggest alternative key values for renderReturnType, and I can update the PR accordingly. Please let me know what else I can do to get this in!

@amnambiar
Copy link
Author

amnambiar commented Jan 24, 2022

Regarding your point 3 above, I do not want the cellData to return a string back to be inserted into the innerHTML. Instead I want the entire insserting of data to be handled within the cellData function itself.

Regarding 4, inorder to faciliate sorting I might have to update the implementation part below
col.render = (data, type, row, meta) => { return createdCellCallback; }
into
col.render = (data, type, row, meta) => { if (type === 'display') { return createdCellCallback; } else {return data} }

@amnambiar
Copy link
Author

Regarding 2, Yes it is a new Key i introduced to faciliate deciding if cellData needs to be invoked then and there using .call or if the cellData needs to be considered as a function to be returned, to be invoked when cell html is to be placed.

@amnambiar
Copy link
Author

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.

4 participants