-
-
Notifications
You must be signed in to change notification settings - Fork 652
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
feat: suggest strategy from template #2340
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
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.
Works fine in manual testing, however I'd consider "slots not props" approach to avoid prop drilling and pass children instead.
onAfterAddStrategy: () => void; | ||
} | ||
|
||
export const AddFromTemplateCard: FC<IAddFromTemplateCardProps> = ({ |
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.
I'd consider adding PresetCard as {children} to avoid prop-drilling title, children, stretegy, Icon, onAfterAddStrategy. More info about this technique: https://www.youtube.com/watch?v=3XaXKiXtNjw&ab_channel=ReactTraining
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.
I got rid of PresetCard
, embedded the code in parent. At this point it was just styling. I couldn't entirely move it to {children}
because of onClick action handler.
About the changes
Add "change request" to buttons in empty environment - adding with "Use template".
Closes 1-348/add-strategy-to-draft-with-use-template
Important files
frontend/src/component/feature/FeatureStrategy/FeatureStrategyEmpty/FeatureStrategyEmpty.tsx
Discussion points