fix: Console error when opening context menu on tree table#2047
fix: Console error when opening context menu on tree table#2047AkshatJawne merged 5 commits intodeephaven:mainfrom
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2047 +/- ##
=======================================
Coverage 46.36% 46.36%
=======================================
Files 673 673
Lines 38780 38779 -1
Branches 9826 9826
=======================================
Hits 17981 17981
+ Misses 20748 20747 -1
Partials 51 51
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| assertNotNull(formatValue); | ||
| resultRow.push( | ||
| formatValue(viewportRow.data.get(c)?.value, this.columns[c]) | ||
| formatValue?.(viewportRow.data.get(c)?.value, this.columns[c]) |
There was a problem hiding this comment.
Seems if formatValue is not defined, there is no point in entering the for loop at all. I would check before the loop.
There was a problem hiding this comment.
Good catch, just tested and it seems like this also works, but definitely more efficient (since we are not going into the loop if its not defined anymore). Pushed change
bmingles
left a comment
There was a problem hiding this comment.
Code changes look good. I confirmed the error no longer shows up when opening the context menu on tree table.
| c += 1 | ||
| ) { | ||
| resultRow.push( | ||
| formatValue(viewportRow.data.get(c)?.value, this.columns[c]) |
There was a problem hiding this comment.
We still want to populate the resultRow array with unformatted values if formatValue is not defined.
With this change, the resultRow ends up empty.
Resolves #2029
Changes Implemented:
I had also considered the approach below, which achieved the same result of removing the exception in the console. Ultimately opted to not use this logic, under the assumption that we are okay with the
resultRowbeingundefined.However, if we want
resultRowto be defined, this approach ensures that it is, via a default function that performs no formatting.