-
Notifications
You must be signed in to change notification settings - Fork 3
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
MC-1656: adding edit & remove section item actions #1231
MC-1656: adding edit & remove section item actions #1231
Conversation
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.
one nit about a test name, but this looks really good!
suggestion to have a curator or two test on dev before merging, but i'll leave that up to you. (we don't have any data in prod yet so i don't think it's too risky to merge now.)
src/curated-corpus/components/SectionDetails/SectionDetails.test.tsx
Outdated
Show resolved
Hide resolved
onMoveToBottom, | ||
onReject, | ||
onRemove, | ||
} = props; |
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.
not needed yet, but a potential thought for the future - would it make sense to abstract the actions a bit more, so this component doesn't need to know about all available actions across all card implementations?
a consideration for if/when we expand the available card actions...
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.
@jpetto yeah good thought! I was thinking about it, we could potentially introduce a new generic type:
interface CardAction {
actionName: string;
icon: React.ReactNode;
onClick: VoidFunction;
}
and maybe update CardActionButtonRowProps
:
interface CardActionButtonRowProps {
cardActionButtonsRight?: CardAction[]; // action buttons aligned to bottom right
cardActionButtonsLeft?: CardAction[]; // action buttons aligned to bottom left
}
I can make a chore ticket to do this!
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.
yeah, i think that's a good ticket to file. thanks!
Goal
Wiring up actions for editing corpus metadata on a
SectionItem
& removing aSectionItem
.onRemove
callback toCardActionButtonRow
to differentiate between theonReject
callback. Using theClearIcon
(X) for the remove button.SectionDetails
component tests.Todos
Reference
Demo
Screen.Recording.2025-02-27.at.13.37.45.mov
Tickets: