-
Notifications
You must be signed in to change notification settings - Fork 71
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
Add Example to Crm Data Components to Demonstrate CrmCardActions
& CrmActionButton
#75
base: main
Are you sure you want to change the base?
Conversation
@aanchalHS you were listed as a suggested reviewer for this PR, but please feel free to tag someone else if this recommendation was incorrect! |
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.
Looks good to me.
Would it be worth showing how to use the onError
callback for at least one of the actions? There isn't an explicit example of that in the docs.
hubspot.extend(({ context, runServerlessFunction, actions }) => ( | ||
<Extension | ||
context={context} | ||
runServerless={runServerlessFunction} | ||
sendAlert={actions.addAlert} | ||
/> | ||
)); |
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.
Probably don't need these if they aren't being used in your extension?
hubspot.extend(({ context, runServerlessFunction, actions }) => ( | |
<Extension | |
context={context} | |
runServerless={runServerlessFunction} | |
sendAlert={actions.addAlert} | |
/> | |
)); | |
hubspot.extend(({ context }) => ( | |
<Extension | |
context={context} | |
/> | |
)); |
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.
It looks like all three props are required for Extension
per its type. Even though they're not used, we may still want to leave them to prevent TS errors
]; | ||
|
||
const Extension = ({ context, runServerless, sendAlert }) => { | ||
const [scenario, setScenario] = useState<any>("engagement"); |
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.
Should this be a string if using ts?
const [scenario, setScenario] = useState<any>("engagement"); | |
const [scenario, setScenario] = useState<string>("engagement"); |
const currentObjectId = context.crm.objectId; | ||
const currentObjectTypeId = context.crm.objectTypeId; | ||
|
||
const actionsToPropertiesMap = { |
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.
Might be worth wrapping this in a useMemo
?
</Text> | ||
|
||
<Flex direction="row" align="end" gap="small"> | ||
<Form> |
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.
Do you need a Form
here?
- Add example of onError for CrmActionsButton - PR Feedback
Summary
This PR creates a new example for the
CrmCardActions
andCrmActionButton
components withincrm-data-components
. This new example is aCrmPropertyList
that dynamically changes the properties displayed and actions button based on a given scenario from a dropdown select. Updates theREADME
with images and documentation as well.