Changeset View
Standalone View
web/chat/message-tooltip.react.js
Show All 9 Lines | |||||
import type { ThreadInfo } from 'lib/types/thread-types.js'; | import type { ThreadInfo } from 'lib/types/thread-types.js'; | ||||
import { | import { | ||||
tooltipButtonStyle, | tooltipButtonStyle, | ||||
tooltipLabelStyle, | tooltipLabelStyle, | ||||
tooltipStyle, | tooltipStyle, | ||||
} from './chat-constants.js'; | } from './chat-constants.js'; | ||||
import css from './message-tooltip.css'; | import css from './message-tooltip.css'; | ||||
import { useSendReaction } from './reaction-message-utils.js'; | import { | ||||
useSendReaction, | |||||
getEmojiKeyboardPosition, | |||||
} from './reaction-message-utils.js'; | |||||
import { useTooltipContext } from './tooltip-provider.js'; | import { useTooltipContext } from './tooltip-provider.js'; | ||||
import { useSelector } from '../redux/redux-utils.js'; | import { useSelector } from '../redux/redux-utils.js'; | ||||
import { type MessageTooltipAction } from '../utils/tooltip-utils.js'; | import type { | ||||
MessageTooltipAction, | |||||
TooltipSize, | |||||
TooltipPositionStyle, | |||||
} from '../utils/tooltip-utils.js'; | |||||
type MessageTooltipProps = { | type MessageTooltipProps = { | ||||
+actions: $ReadOnlyArray<MessageTooltipAction>, | +actions: $ReadOnlyArray<MessageTooltipAction>, | ||||
+messageTimestamp: string, | +messageTimestamp: string, | ||||
+alignment?: 'left' | 'center' | 'right', | +tooltipPositionStyle: TooltipPositionStyle, | ||||
+tooltipSize: TooltipSize, | |||||
+item: ChatMessageInfoItem, | +item: ChatMessageInfoItem, | ||||
+threadInfo: ThreadInfo, | +threadInfo: ThreadInfo, | ||||
}; | }; | ||||
function MessageTooltip(props: MessageTooltipProps): React.Node { | function MessageTooltip(props: MessageTooltipProps): React.Node { | ||||
const { | const { | ||||
actions, | actions, | ||||
messageTimestamp, | messageTimestamp, | ||||
alignment = 'left', | tooltipPositionStyle, | ||||
tooltipSize, | |||||
item, | item, | ||||
threadInfo, | threadInfo, | ||||
} = props; | } = props; | ||||
const { messageInfo, reactions } = item; | const { messageInfo, reactions } = item; | ||||
const { alignment = 'left' } = tooltipPositionStyle; | |||||
const [activeTooltipLabel, setActiveTooltipLabel] = React.useState<?string>(); | const [activeTooltipLabel, setActiveTooltipLabel] = React.useState<?string>(); | ||||
const { renderEmojiKeyboard } = useTooltipContext(); | const { shouldRenderEmojiKeyboard } = useTooltipContext(); | ||||
const [emojiKeyboardNode, setEmojiKeyboardNode] = React.useState(null); | |||||
const emojiKeyboardRef = React.useRef(); | |||||
// Using an effect to set state is generally considered an anti-pattern in | |||||
// React, and should be avoided. In this case it's necessary because we | |||||
// want to set the ref of the emoji keyboard after <Picker /> initially | |||||
// renders onto the DOM so we can get the correct height and width | |||||
ginsu: I added a comment here to help with readability and better address @ashoat's coupling concerns. | |||||
ashoatUnsubmitted Not Done Inline ActionsThis comment is misleading... it implies that refs get passed in before a component renders to the DOM. This is not correct... the ref gets passed in after the component first renders. What's happening here is that <Picker /> has some custom behavior where it renders initially with height: 0 and width: 0, and then resizes itself ashoat: This comment is misleading... it implies that refs get passed in before a component renders to… | |||||
ashoatUnsubmitted Not Done Inline ActionsThe real reason this is happening is because emoji-mart doesn't actually render itself until a React.useEffect here, which won't get run until after our ref is set. This is why we appear to need to use a React.useEffect ourselves as well. (The idiomatic thing for emoji-mart to do would've been to render itself immediately instead of in an effect... but if the effect is strictly necessary, at least they should have a callback so that the parent component can know when rendering is complete...) ashoat: The real reason this is happening is because `emoji-mart` doesn't actually render itself until… | |||||
React.useEffect(() => { | |||||
if (emojiKeyboardRef.current && shouldRenderEmojiKeyboard) { | |||||
setEmojiKeyboardNode(emojiKeyboardRef.current); | |||||
} | |||||
}, [shouldRenderEmojiKeyboard]); | |||||
ashoatUnsubmitted Not Done Inline Actions
ashoat: 1. Your comment doesn't address the coupling. The baseline thing I suggested above is to add a… | |||||
const messageActionButtonsContainerClassName = classNames( | const messageActionButtonsContainerClassName = classNames( | ||||
css.messageActionContainer, | css.messageActionContainer, | ||||
css.messageActionButtons, | css.messageActionButtons, | ||||
); | ); | ||||
const messageTooltipButtonStyle = React.useMemo(() => tooltipButtonStyle, []); | const messageTooltipButtonStyle = React.useMemo(() => tooltipButtonStyle, []); | ||||
▲ Show 20 Lines • Show All 57 Lines • ▼ Show 20 Lines | const tooltipTimestamp = React.useMemo(() => { | ||||
} | } | ||||
return ( | return ( | ||||
<div className={css.messageTooltipLabel} style={messageTooltipLabelStyle}> | <div className={css.messageTooltipLabel} style={messageTooltipLabelStyle}> | ||||
{messageTimestamp} | {messageTimestamp} | ||||
</div> | </div> | ||||
); | ); | ||||
}, [messageTimestamp, messageTooltipLabelStyle]); | }, [messageTimestamp, messageTooltipLabelStyle]); | ||||
const emojiKeyboardPosition = React.useMemo( | |||||
() => | |||||
getEmojiKeyboardPosition( | |||||
emojiKeyboardNode, | |||||
tooltipPositionStyle, | |||||
tooltipSize, | |||||
), | |||||
[emojiKeyboardNode, tooltipPositionStyle, tooltipSize], | |||||
); | |||||
const emojiKeyboardPositionStyle = React.useMemo(() => { | |||||
if (!emojiKeyboardPosition) { | |||||
return null; | |||||
} | |||||
return { | |||||
bottom: emojiKeyboardPosition.bottom, | |||||
left: emojiKeyboardPosition.left, | |||||
}; | |||||
}, [emojiKeyboardPosition]); | |||||
const nextLocalID = useSelector(state => state.nextLocalID); | const nextLocalID = useSelector(state => state.nextLocalID); | ||||
const localID = `${localIDPrefix}${nextLocalID}`; | const localID = `${localIDPrefix}${nextLocalID}`; | ||||
const sendReaction = useSendReaction(messageInfo.id, localID, threadInfo.id); | const sendReaction = useSendReaction(messageInfo.id, localID, threadInfo.id); | ||||
const onEmojiSelect = React.useCallback( | const onEmojiSelect = React.useCallback( | ||||
emoji => { | emoji => { | ||||
const reactionInput = emoji.native; | const reactionInput = emoji.native; | ||||
const viewerReacted = reactions[reactionInput] | const viewerReacted = reactions[reactionInput] | ||||
? reactions[reactionInput].viewerReacted | ? reactions[reactionInput].viewerReacted | ||||
: false; | : false; | ||||
const action = viewerReacted ? 'remove_reaction' : 'add_reaction'; | const action = viewerReacted ? 'remove_reaction' : 'add_reaction'; | ||||
sendReaction(reactionInput, action); | sendReaction(reactionInput, action); | ||||
}, | }, | ||||
[sendReaction, reactions], | [sendReaction, reactions], | ||||
); | ); | ||||
const emojiKeyboard = React.useMemo(() => { | const emojiKeyboard = React.useMemo(() => { | ||||
if (!renderEmojiKeyboard) { | if (!shouldRenderEmojiKeyboard) { | ||||
return null; | return null; | ||||
} | } | ||||
return <Picker data={data} onEmojiSelect={onEmojiSelect} />; | |||||
}, [onEmojiSelect, renderEmojiKeyboard]); | return ( | ||||
<div | |||||
ref={emojiKeyboardRef} | |||||
style={emojiKeyboardPositionStyle} | |||||
className={css.emojiKeyboard} | |||||
> | |||||
<Picker data={data} onEmojiSelect={onEmojiSelect} /> | |||||
</div> | |||||
); | |||||
}, [emojiKeyboardPositionStyle, onEmojiSelect, shouldRenderEmojiKeyboard]); | |||||
const messageTooltipContainerStyle = React.useMemo(() => tooltipStyle, []); | const messageTooltipContainerStyle = React.useMemo(() => tooltipStyle, []); | ||||
const containerClassName = classNames({ | const containerClassName = classNames({ | ||||
[css.container]: true, | |||||
[css.containerLeftAlign]: alignment === 'left', | |||||
[css.containerCenterAlign]: alignment === 'center', | |||||
}); | |||||
const messageTooltipContainerClassNames = classNames({ | |||||
[css.messageTooltipContainer]: true, | [css.messageTooltipContainer]: true, | ||||
[css.leftTooltipAlign]: alignment === 'left', | [css.leftTooltipAlign]: alignment === 'left', | ||||
[css.centerTooltipAlign]: alignment === 'center', | [css.centerTooltipAlign]: alignment === 'center', | ||||
[css.rightTooltipAlign]: alignment === 'right', | [css.rightTooltipAlign]: alignment === 'right', | ||||
}); | }); | ||||
return ( | return ( | ||||
<div className={containerClassName}> | <> | ||||
{emojiKeyboard} | {emojiKeyboard} | ||||
<div | <div className={containerClassName} style={messageTooltipContainerStyle}> | ||||
className={messageTooltipContainerClassNames} | |||||
style={messageTooltipContainerStyle} | |||||
> | |||||
<div style={messageTooltipTopLabelStyle}>{tooltipLabel}</div> | <div style={messageTooltipTopLabelStyle}>{tooltipLabel}</div> | ||||
{tooltipButtons} | {tooltipButtons} | ||||
{tooltipTimestamp} | {tooltipTimestamp} | ||||
</div> | </div> | ||||
</div> | </> | ||||
); | ); | ||||
} | } | ||||
export default MessageTooltip; | export default MessageTooltip; |
I added a comment here to help with readability and better address @ashoat's coupling concerns. I also think this comment would be good to caution future devs who work in this file and see this code to not make this a recurring pattern in our codebase