Skip to content

feat(unified-share-modal): add custom avatars click handler - #3688

Merged
mergify[bot] merged 1 commit into
box:masterfrom
tnastula:custom-avatars-click-handler
Oct 1, 2024
Merged

mergify[bot] merged 1 commit into
box:masterfrom
tnastula:custom-avatars-click-handler

Conversation

@tnastula

Copy link
Copy Markdown
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

@tjuanitas tjuanitas left a comment

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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
Copy Markdown
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

// 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
Copy Markdown
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
Copy Markdown
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 83649c6 to 0be4f58 Compare October 1, 2024 07:49
@tnastula
tnastula requested a review from tjuanitas October 1, 2024 07:50
@tnastula
tnastula force-pushed the custom-avatars-click-handler branch from 0be4f58 to af4560e Compare October 1, 2024 07:50
@tnastula
tnastula force-pushed the custom-avatars-click-handler branch from af4560e to 30e2f1a Compare October 1, 2024 12:27
@tnastula

tnastula commented Oct 1, 2024

Copy link
Copy Markdown
Contributor Author

@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
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