Clarity: Error on auto expanding row details in datagrid

[vraddy]

Describe the bug

Have a set of rows with predefined values 'row.expand'(true or false) to expand rows automatically on initial render. Getting below console error while auto-expanding rows in clarity Datagrid.
ERROR DOMException: Failed to execute 'animate' on 'Element': Partial keyframes are not supported. at DatagridRowExpandAnimation.push../node_modules/@clr/angular/esm5/clr-angular.js.DatagridRowExpandAnimation.run

To be more specific the error occure in following line, due to oldHeight is 'NaN' and this will be 'NaNpx' which is invalid value for animation.
DatagridRowExpandAnimation.prototype.run = { ... this.running = this.el.nativeElement.animate({ height: [this.oldHeight + 'px', newHeight + 'px'], overflowY: ['hidden', 'hidden'], easing: 'ease-in-out' }, { duration: 200 }); ... }

How to reproduce

https://stackblitz.com/edit/clarity-light-theme-v1-0-88t8rr

Steps to reproduce the behavior:

1. Bind data to datagrid which has property to expand row detail example 'row.expand (true | false)'
2. On initial render datagrid logs an error in console
   for large data set(around 50 records) browser will become unresponsive.

Expected behavior

Allow to automitically expand rows on initial render

Versions

App

• Angular: 6
• Node: 8
• Clarity: 0.13

Device:

• Type: Laptop
• OS: Windows
• Browser Chrome
• Version 70.0.3538.110

[Jinnie]

Note (to whoever is taking care of this issue):
On the website docs we state that the directive takes no input:
To make a row expandable, you need to put a <clr-dg-row-detail> component inside your row, and add a *clrIfExpanded structural directive on it. This directive doesn't take any input, it is here for 2 reasons: .........
It's probably not very precise, looking at the code. We may need to update the documentation and provide examples and tests if we're supporting input bindings here.

[youdz]

The good news is that the app still works, you can expand/collapse the details fine.
The bad news is that I think it's a bug due to the weird lifecycle of our datagrid rendering, which indeed seems to have a problem with animations. To be more precise, I believe the details are properly instantiated at the correct time, but they're instantiated detached from the DOM, which causes the error mentioned in the bug report. It'll probably take quite a bit of investigation to fix.

[vraddy]

@Jinnie Sorry, yes doc mentioned not to use inputs on this clrIfExpanded directive but I found this way of using in some other place(SO I believe), never thought it could give other problem.

@Jinnie @youdz thanks for your response. looking forward to seeing this fix in the future release.
for time being I managed to solve this with small workaround..
https://stackblitz.com/edit/clarity-light-theme-v013-jpj7jk
please feel free if you have any inputs on this.

Cheers

[youdz]

Yes, that's a reasonable workaround. You get the animation on init, but it doesn't seem to be that bad. We still want to fix this error as it's a regression. 😁

[kumars17]

I am facing the same issue and no workaround is helping in this. Any plan when this is getting fixed?

[vraddy]

@Jinnie
Thank you for the fix, but I see this problem is not solved, still its throw same error. after debug found that its failed to calculate _newHight_ in _run()_ function.

[Jinnie]

Hi @veereshradder
Please provide an updated stackblitz that is failing as you describe. The initially observed issue is covered with a test and it should really be failing the same. But it's possible that we've not covered a similar case.
Thanks!

[mattmutt]

Please patch newHeight correctly in the run() method. ( around line 79)

the latest patch addresses one area, but neglected the potential for getting a NaN again in the run method. where newHeight is calculated again.

To be concise:
the new patch concedes to the fact that newHeight may at times return a non number. Therefore when cast to a number, it becomes NaN

when a NaN is concatenated with a string such as px for pixels, the result is NaNpx . Obviously this would fail when the animate function is invoked.

The class in row-expand-animation.ts needs to be comprehensively patched. Therefore examine the flow carefully in the run() method. It is left vulnerable to this same defect

[gnomeontherun]

@mattmutt Do you have an updated example showing this issue? If so, please open a new bug report with details for us so we don't lose track of this old issue.

[github-actions[bot]]

Hi there 👋, this is an automated message. To help Clarity keep track of discussions, we automatically lock closed issues after 14 days. Please look for another open issue or open a new issue with updated details and reference this one as necessary.