-
-
Notifications
You must be signed in to change notification settings - Fork 24
Fix checking for cell renderer function for table tab navigation #1313
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
base: main
Are you sure you want to change the base?
Conversation
Teh actions column and a column with an Id have special behavior and when built the function name changes, so we attached the name to the prototype to gain access to the original name to detect when to have special behavior
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes the detection of specific cell renderer functions (IdLinkRenderer and ActionRenderer) for handling special keyboard navigation behavior in data tables. The issue was that minification/bundling changes function names, breaking the name-based detection.
- Changed from checking
args.column.renderCell?.name
toargs.column.renderCell?.prototype?.displayName
- Added
prototype.displayName
properties to bothIdLinkRenderer
andActionRenderer
functions
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
File | Description |
---|---|
libs/ui/src/lib/data-table/useDataTable.tsx | Updated renderer detection logic to use prototype.displayName instead of function.name |
libs/ui/src/lib/data-table/DataTableRenderers.tsx | Added prototype.displayName properties to IdLinkRenderer and ActionRenderer |
IdLinkRenderer.prototype = { | ||
displayName: 'IdLinkRenderer', | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assigning to a function's prototype property can override its existing prototype chain. Consider using a safer approach like adding a static property (IdLinkRenderer.displayName = 'IdLinkRenderer') or using a symbol/WeakMap for function identification.
IdLinkRenderer.prototype = { | |
displayName: 'IdLinkRenderer', | |
}; | |
IdLinkRenderer.displayName = 'IdLinkRenderer'; |
Copilot uses AI. Check for mistakes.
ActionRenderer.prototype = { | ||
displayName: 'ActionRenderer', | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assigning to a function's prototype property can override its existing prototype chain. Consider using a safer approach like adding a static property (ActionRenderer.displayName = 'ActionRenderer') or using a symbol/WeakMap for function identification.
ActionRenderer.prototype = { | |
displayName: 'ActionRenderer', | |
}; | |
ActionRenderer.displayName = 'ActionRenderer'; |
Copilot uses AI. Check for mistakes.
@@ -301,11 +301,11 @@ export function useDataTable<T = RowWithKey>({ | |||
function handleCellKeydown(args: CellKeyDownArgs<T, unknown>, event: CellKeyboardEvent) { | |||
try { | |||
// Handle renderer-specific keyboard interactions | |||
if (args.column.renderCell?.name === 'IdLinkRenderer' && handleIdLinkRendererKeydown(event)) { | |||
if (args.column.renderCell?.prototype?.displayName === 'IdLinkRenderer' && handleIdLinkRendererKeydown(event)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The nested optional chaining with prototype access is fragile. If the prototype approach changes, consider using a more robust identification method like a static property or instanceof checks.
Copilot uses AI. Check for mistakes.
Teh actions column and a column with an Id have special behavior and when built the function name changes, so we attached the name to the prototype to gain access to the original name to detect when to have special behavior
Teh actions column and a column with an Id have special behavior and when built the function name changes, so we attached the name to the prototype to gain access to the original name to detect when to have special behavior