-
-
Notifications
You must be signed in to change notification settings - Fork 7k
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
Rewrite AvatarOverlay component with React hooks #24543
Conversation
49c7ae3
to
d9844b0
Compare
hovering: false, | ||
}; | ||
|
||
handleMouseEnter = () => { |
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.
Both event handler function object of handleMouseEnter and handleMouseLeave are unused this component.
|
||
return ( | ||
<div className='account__avatar-overlay' style={{ width: size, height: size }}> | ||
<div className='account__avatar-overlay-base'><Avatar animate={hovering || animate} account={account} size={baseSize} /></div> |
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.
No need to pass animate arguments to Avatar component why they have hovering state and its event handler in themself.
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.
Looks like the event handlers were not hooked up but I think the intent was that both avatars would play at the same time when the overlay was hovered.
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.
Looks like the event handlers were not hooked up but I think the intent was that both avatars would play at the same time when the overlay was hovered.
I seem so too.
I will fix it
This movie is behavior of current main branch(e5c0b16).
admin.@admin@localhost_3000.-.Mastodon.Dev.-.Google.Chrome.2023-04-16.14-58-24.mp4
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 made a commit.
b1397f8
admin.@admin@localhost_3000.-.Mastodon.Dev.-.Google.Chrome.2023-04-16.16-59-09.mp4
static propTypes = { | ||
account: ImmutablePropTypes.map.isRequired, | ||
friend: ImmutablePropTypes.map.isRequired, | ||
animate: PropTypes.bool, |
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.
animate argument is unused.
statusAvatar = <AvatarOverlay account={status.get('account')} friend={account} />; |
mastodon/app/javascript/mastodon/features/account_timeline/components/moved_note.jsx
Line 27 in f05fb51
<div className='detailed-status__display-avatar'><AvatarOverlay account={to} friend={from} /></div> |
const component = renderer.create(<AvatarOverlay account={account} friend={friend} />); |
d9844b0
to
d770fd2
Compare
For playing same time, do not attach event handlers to inner elements.
d770fd2
to
b1397f8
Compare
This PR rewrites AvatarOverlay component with React hooks.
And fix animation behavior when component hoverd.