Unplanned
Last Updated: 21 Nov 2019 10:35 by ADMIN
Stuart
Created on: 19 Nov 2019 10:39
Type: Bug Report
1
Error in type bindings for KendoGrid rowRender

Hi Guys,

In GridProps for KendoReact Grid there is an error in the type information:

Found here: https://www.telerik.com/kendo-react-ui/components/grid/api/GridProps/#toc-rowrender

In the GridProps.d.ts file it is:

    /**
     * Fires when a row is about to be rendered. Overrides the default appearance of the row.
     */
    rowRender?: (row: React.ReactElement<HTMLTableRowElement>, props: GridRowProps) => React.ReactNode;

This should be changed to:

rowRender?: (row: React.ReactElement<React.HtmlProps<HTMLTableRowElement>>, props: GridRowProps) => React.ReactNode;

 

Note that the React.ReactElement definition is:

    interface ReactElement<P = any, T extends string | JSXElementConstructor<any> = string | JSXElementConstructor<any>> {
        type: T;
        props: P;
        key: Key | null;
    }

 

So the first type parameter should be the props typ.

I noted this while making use of it. After changing it it behaves sensibly. Worked around in my case by using 'as any' to void the type clash.

Cheers,

Stuart

5 comments
ADMIN
Stefan
Posted on: 21 Nov 2019 10:35

Hello, Stuart,

Thank you for the details information.

We will research this and change it to the most accurate type, so such an issue does not occur.

The item is also transferred to GitHub. so you can track it there as well if it is a preferred place:

https://github.com/telerik/kendo-react/issues/387

Additionally, I have updated your Telerik points for bringing this to our attention.

Regards,
Stefan
Progress Telerik

Do you want to have your say when we set our development plans? Do you want to know when a feature you care about is added or when a bug fixed? Explore the Telerik Feedback Portal and vote to affect the priority of the items
Stuart
Posted on: 21 Nov 2019 09:13

Hi Stefan,

You are correct that it does have a 'style' property, however it typed as a HTML DOM style property and not as a React CSSProperties - which is what is should be. In this case I think the names are the same so might not matter. Possibly a bigger problem is that I could not correctly subscribe to events as per below.

This is what happens if I restore the original bindings. I've just highlighted the detected type of my copy of row.props.

(Note that row.props is typed as a HTMLTableRowElement when it is expected to be a props object.)

onDoubleClick is correct (and works perfectly) - but I am offered instead the DOM event handler names:

If I debug one step higher to where GridRow calls the function:

defaultRendering.props is not a DOM element, it is a React Props.

Having a play to see why you are not getting the error, I tried this:

It seems that when you use JSX to create the tr, the type it gives you is:

Which is basically React.ReactElement<any> which will silently convert to React.ReactElement<HTMLTableRowElement>:

Anyhoo - in my case the workaround is to cheat the type system with an 'as any', but I thought it would be good to update at the source. I hope my comments make sense and there isn't something I'm missing.

Warm Regards,

Stuart

ADMIN
Stefan
Posted on: 21 Nov 2019 08:24

Hello,

Thank you for the additional details.

I tested this and with the current type, the style prop is valid for the element. Did I miss something?

Regards,
Stefan
Progress Telerik

Do you want to have your say when we set our development plans? Do you want to know when a feature you care about is added or when a bug fixed? Explore the Telerik Feedback Portal and vote to affect the priority of the items
Stuart
Posted on: 20 Nov 2019 22:16

Hi Stefan,

I believe for basic use cases the incorrect type bindings would not cause a problem.

As I wanted to modify the associated styles for the row, I was not able to as row.styles did not exist in the 'current' bindings.

I have pasted my code below if it is any interest to you guys.

Cheers,

----

 

    private renderRow(row: ReactElement<React.HTMLProps<HTMLTableRowElement>>, props: GridRowProps): React.ReactNode {
        const trProps: React.HTMLProps<HTMLTableRowElement> = {
            ...row.props
        };

        if (this.props.onRowDoubleClick) {
            trProps.onDoubleClick = (evt: React.MouseEvent<HTMLTableRowElement>) => {
                evt.preventDefault();

                if (this.props.idField && this.props.onRowDoubleClick) {
                    let id = props.dataItem[this.props.idField];

                    this.props.onRowDoubleClick(id);
                }
            };
        }

        if (this.props.onRowClick) {
            if (!trProps.style) {
                trProps.style = {};
            }

            trProps.style.cursor = 'pointer';
        }

        return React.cloneElement(row, trProps, row.props.children);
    }

ADMIN
Stefan
Posted on: 20 Nov 2019 14:08

Hello, Stuart,

Thank you for the report.

We have tested it and can confirm that it is working.

Before we proceed with the change, could you please let us know the issue that occurs with the other type?

Our components and internal demos are built on TypeScript and we have not encountered any issue with the current type. 

Regards,
Stefan
Progress Telerik

Do you want to have your say when we set our development plans? Do you want to know when a feature you care about is added or when a bug fixed? Explore the Telerik Feedback Portal and vote to affect the priority of the items