Skip to content
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(unified-share-modal): add custom avatars click handler #3688

Merged
merged 1 commit into from
Oct 1, 2024

Conversation

tnastula
Copy link
Contributor

This PR adds a handler to USM that if defined will trigger alternate, custom action upon clicking avatars

@tnastula tnastula requested a review from a team as a code owner September 30, 2024 11:06
tjuanitas
tjuanitas previously approved these changes Oct 1, 2024
Copy link
Contributor

@tjuanitas tjuanitas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some nits

Comment on lines 374 to 375
/** A custom action to be invoked instead of default behavior when collaborators avatars are clicked */
handleCollaboratorAvatarsClick?: () => void,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this list of sorted alphabetically

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch, adjusted

@@ -371,6 +371,8 @@ type BaseUnifiedShareProps = CollaboratorAvatarsTypes &

// Prop types for the Unified Share Modal
export type USMProps = BaseUnifiedShareProps & {
/** A custom action to be invoked instead of default behavior when collaborators avatars are clicked */
handleCollaboratorAvatarsClick?: () => void,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: for the prop names of callback functions, we usually use the "on" prefix. "handle" would be used instead when we're defining a function/method

e.g. onCollaboratorListClick

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion, adjusted

@tnastula tnastula force-pushed the custom-avatars-click-handler branch from af4560e to 30e2f1a Compare October 1, 2024 12:27
@tnastula
Copy link
Contributor Author

tnastula commented Oct 1, 2024

@tjuanitas I don't know why Chromatic fails. Those are not my changes

@mergify mergify bot merged commit c034de4 into box:master Oct 1, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants