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

"undefined" values of markdown columns during grouping #171

Closed
roman-kachanovsky opened this issue May 4, 2023 · 7 comments · Fixed by #174
Closed

"undefined" values of markdown columns during grouping #171

roman-kachanovsky opened this issue May 4, 2023 · 7 comments · Fixed by #174

Comments

@roman-kachanovsky
Copy link

If table contains columns with cellRenderer="markdown" these columns show "undefined" values in group headers regardless of which columns are used for grouping.

Code snippet:

import dash_ag_grid as dag
import pandas as pd
from dash import Dash, html


app = Dash(__name__)

df = pd.read_csv(
    "https://raw.githubusercontent.com/plotly/datasets/master/ag-grid/olympic-winners.csv"
)

columnDefs = [
    {"field": "country", "rowGroup": True, "hide": True},
    {"field": "sport"},
    {"field": "year"},
    {"field": "markdown_link", "cellRenderer": "markdown"},
    {"field": "html_link", "cellRenderer": "markdown"},
    {"field": "age"},
]

rows = df.to_dict("records")
for row in rows:
    row["markdown_link"] = "[Markdown Link](https://example.com)"
    row["html_link"] = "<a href='https://example.com'>HTML Link</a>"

app.layout = html.Div(
    [
        dag.AgGrid(
            columnDefs=columnDefs,
            rowData=rows,
            columnSize="sizeToFit",
            defaultColDef={"resizable": True, "sortable": True, "filter": True},
            enableEnterpriseModules=True,
            dangerously_allow_code=True,
        ),
    ],
    style={"margin": 20},
)

if __name__ == "__main__":
    app.run_server(debug=True)

Result:
image

@BSd3v
Copy link
Collaborator

BSd3v commented May 4, 2023

Yes.

A group renderer is different than a regular cellRenderer.

A workaround is to use dcc_markdown as a custom renderer designed to only render when there is a params.value.

@roman-kachanovsky
Copy link
Author

A workaround is to use dcc_markdown as a custom renderer designed to only render when there is a params.value.

Can you share an example of dcc_markdown renderer usage? I can't find it in documentations.

@BSd3v
Copy link
Collaborator

BSd3v commented May 4, 2023

You can check out examples here.

Also, could you please post this on the community forums?
https://community.plotly.com/c/python/25

@roman-kachanovsky
Copy link
Author

I've played a bit with custom renderers and what I got. The snippet below works for pure Markdown but ignores dangerously_allow_html property and renders empty strings when it's true:

let dagComponentFuncs = window.dashAgGridComponentFunctions = window.dashAgGridComponentFunctions || {};

dagComponentFuncs.DCC_Markdown = function (props) {
	if (props.value == null) {
		return "";
	}
	
	return React.createElement(
		window.dash_core_components.Markdown,
		{
			dangerously_allow_html: props.dangerously_allow_code,
		}, 
		props.value
	);
};

But it's not suitable for us because we use markdown renderer to pass quite complicated HTML blocks like preview images, icons, styled spans into our table. We can put HTML code directly into table cell using approach like this:

let dagComponentFuncs = window.dashAgGridComponentFunctions = window.dashAgGridComponentFunctions || {};

dagComponentFuncs.htmlRenderer = function (props) {
	if (props.value == null) {
		return "";
	}
	
	return React.createElement('div', {
		dangerouslySetInnerHTML: { __html: props.value }
	});
};

And it works, but I feel that both ways (markdown / htmlRenderer) look like kinda dirty hack. Probably, we have to implement custom renderers for each case, e.g. previewRenderer for preview column. It would be safe and clean. But generic approach like markdown renderer is also has some positive sides. IDK, we'll think about it. But I think inner markdown renderer shouldn't force to find alternative ways because of conflict with group renderer. Especially since the conflict did not exist in Dash Enterprise 1.3.0 version of AGG package. Markdown renderer and grouping work well in that version together.

@BSd3v
Copy link
Collaborator

BSd3v commented May 5, 2023

Hello @roman-kachanovsky,

DCC_Markdown should be working with allowing the html... which it is not, this is actually a separate issue.

I can update the cellRenderer to check for the value, but even the original renderer from 1.3 did not, so it is interesting that this is returning "undefined".

I really recommend trying to stray away from using raw_html inside of the grid, there is tons of flexibility with all the custom components that you can make.

For example:

dagcomponentfuncs.stockLink = function (props) {
    return React.createElement('a',
    {
        href: 'https://finance.yahoo.com/quote/' + props.value,
        target: props.value
    }, props.value)
}

This creates a link to yahoo with just the ticker being in the cell, the major benefit of this is that you can export your data to csv and excel without all the markdown or html info.

Here is an easier way to do the markdown component as well:

dagcomponentfuncs.DCC_Markdown = function (props) {
	if (props.value == null) {
		return "";
	}

	return React.createElement(
		window.dash_core_components.Markdown,
		{
			dangerously_allow_html: props.dangerously_allow_code,
			...props.colDef.cellRendererParams
		},
		props.value
	);
};

Notice the ...props.colDef.cellRendererParams, this breaks the object into its individual key-value pairs, allowing for more flexible coding on your end and being cleaner in the value. Once DCC_Markdown is fixed, you could even pass it the dangerously_allow_html directly from the cellRendererParams without exposing the rest of the grid to dangerously_allow_code.

To pass the cellRendererParams, you would do something like this:

{"headerName": "Link", "field": "link", "cellRenderer": "DCC_Markdown",
     "cellRendererParams": {"link_target":"_blank"}},

@BSd3v BSd3v linked a pull request May 5, 2023 that will close this issue
@roman-kachanovsky
Copy link
Author

Thanks @BSd3v I really appreciate your advices. Actually, we are happy that dash-ag-grid now provides an opportunity to extend its functionality using js functions in natural way. We had some ugly formatters before, for example, and even broken table filter due we couldn't attach correct comparator, but now we can fix and improve all that stuff.

@BSd3v
Copy link
Collaborator

BSd3v commented May 6, 2023

You're welcome. We tried to give as much freedom as because AG grid has so many features.

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 a pull request may close this issue.

2 participants