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

Added CellRenderers #748

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

Added CellRenderers #748

wants to merge 3 commits into from

Conversation

svpace
Copy link

@svpace svpace commented Jul 21, 2017

CellRenderers can be used to define cell level customization to the default Table Renderer. Workaround for issue #707.

@nicolaskruchten
Copy link
Owner

Thanks for this PR, and apologies for taking so long to respond!

I'm weighing the complexity of merging this PR and maintaining the documentation etc, as compared to recommending that developers do something similar to the Heatmap and Barchart renderers, which basically walk the output of the Table Renderer and tweak the DOM.

@svpace
Copy link
Author

svpace commented Sep 22, 2017

I've struggled with this decision myself. I appreciate the separation of concerns of the layered renderers. But in the end, I was frequently writing encoded "hints" to the cells just remove and translate then to the good stuff in the next step. It felt inefficient and it was harder to reason about. Particularly when you start doing "fancy stuff" with aggregators and labels, using plain text to bridge the communication with the renderers was a pita.

I believe that the concept is easy enough to explain, its basically a hook to the last mile of the table renderer and the cell renderers are just functions to map some context to an html element. As soon as I figure out how to make contributions to the wiki I could write a couple of paragraphs explaining the feature.

@nicolaskruchten
Copy link
Owner

nicolaskruchten commented Sep 22, 2017 via email

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