-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
chore: Refactor Edit Album Modal #18653
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?
chore: Refactor Edit Album Modal #18653
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.
Nice!
There's more where that's coming from! |
const handleEdit = async (album: AlbumResponseDto) => { | ||
await modalManager.show(EditAlbumForm, { | ||
album, | ||
onEditSuccess: successEditAlbumInfo, |
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.
Just looked at this again and you should get rid of onEditSuccess
. EditAlbumForm should have an onClose(album?: ...) => void
; if album is provided it was a success, otherwise it was cancelled. That's how we treat those generally now
}); | ||
if (editedAlbum) { | ||
successEditAlbumInfo(editedAlbum); | ||
} | ||
closeAlbumContextMenu(); |
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 want to close the context menu before the modal is opened? That's how it was before. (kind of, because before the operation before wasn't blocking)
Description
Refactored EditAlbumModal to use the new modalManager