-
Notifications
You must be signed in to change notification settings - Fork 8
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
AG-10486 Contextmenu callback type-safety and docs #1614
AG-10486 Contextmenu callback type-safety and docs #1614
Conversation
cc0d975
to
546335e
Compare
packages/ag-charts-community/src/chart/interaction/contextMenuRegistry.ts
Outdated
Show resolved
Hide resolved
@@ -33,17 +32,17 @@ export class ContextMenu extends _ModuleSupport.BaseModuleInstance implements _M | |||
/** | |||
* Extra menu actions with a label and callback. | |||
*/ | |||
public extraActions: Array<ContextMenuAction> = []; | |||
public extraActions: NonNullable<AgContextMenuOptions['extraActions']> = []; |
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.
nit: I think we're generally trying to separate options types and modules/features types, even if it produces a bit of duplication?
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.
Correct.
We still do this: The internal state is stored in ContextMenu.groups
, whereas ContextMenu.extraActions
stores the input user options.
private readonly groups: ContextMenuGroups; |
type ContextMenuGroups = { |
The ContextMenu.groups
property contains some duplicated values like label
and action
, but it also contains internal meta-data like id?
and type
:
ag-charts/packages/ag-charts-community/src/chart/interaction/contextMenuRegistry.ts
Line 25 in 1a7fe5c
export type ContextMenuAction = { |
The legend context menu callbacks were apparently being called with the incorrection parameter type. This change solves that and enforces type- safely when invoking context menu callback.
Note: This is a breaking change because the event.type string in the option contract is changing.
This fixes extraNodeActions and extraLegendItemActions.
546335e
to
b297274
Compare
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.
LGTM
https://ag-grid.atlassian.net/browse/AG-10486