From e3aad31f8652280202758d2c11416f6fc82309e6 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Wed, 4 Aug 2021 16:16:01 -0400 Subject: [PATCH 1/5] MentionWarnings [nfc]: Convert to function component. This one's a little unusual because of the design where imperative methods are exposed for a parent of MentionWarnings to call. Use `forwardRef` and `useImperativeHandle`, as the Hooks way to implement this design; see https://reactjs.org/docs/hooks-reference.html#useimperativehandle. Note that "forwardRef" already appears near the bottom of MentionWarnings.js. There, we're telling `connect` to forward the ref to the component wrapped by the HOC. That one will disappear in the next commit, when we stop using `connect`. But the second one will remain, since that's our ticket to allowing callers to take a `ref` to our function component; see the doc linked above, and https://reactjs.org/docs/refs-and-the-dom.html#refs-and-function-components. --- src/compose/MentionWarnings.js | 255 ++++++++++++++++----------------- 1 file changed, 126 insertions(+), 129 deletions(-) diff --git a/src/compose/MentionWarnings.js b/src/compose/MentionWarnings.js index 17dadf627ff..70e694ef1f6 100644 --- a/src/compose/MentionWarnings.js +++ b/src/compose/MentionWarnings.js @@ -1,18 +1,9 @@ /* @flow strict-local */ -import React, { PureComponent } from 'react'; import { connect } from 'react-redux'; +import React, { useState, useCallback, useContext, forwardRef, useImperativeHandle } from 'react'; -import type { - Auth, - Stream, - Dispatch, - Narrow, - UserOrBot, - Subscription, - GetText, - UserId, -} from '../types'; +import type { Auth, Stream, Dispatch, Narrow, UserOrBot, Subscription, UserId } from '../types'; import { TranslationContext } from '../boot/TranslationProvider'; import { getAllUsersById, getAuth } from '../selectors'; import { is1to1PmNarrow } from '../utils/narrow'; @@ -22,10 +13,6 @@ import { showToast } from '../utils/info'; import MentionedUserNotSubscribed from '../message/MentionedUserNotSubscribed'; import { makeUserId } from '../api/idTypes'; -type State = {| - unsubscribedMentions: Array, -|}; - type SelectorProps = {| auth: Auth, allUsersById: Map, @@ -39,137 +26,147 @@ type Props = $ReadOnly<{| ...SelectorProps, |}>; -class MentionWarnings extends PureComponent { - static contextType = TranslationContext; - context: GetText; - - state = { - unsubscribedMentions: [], - }; - - /** - * Tries to parse a user object from an @-mention. - * - * @param completion The autocomplete option chosend by the user. - See JSDoc for AutoCompleteView for details. - */ - getUserFromMention = (completion: string): UserOrBot | void => { - const { allUsersById } = this.props; - - const unformattedMessage = completion.split('**')[1]; - - // We skip user groups, for which autocompletes are of the form - // `**`, and therefore, message.split('**')[1] - // is undefined. - if (unformattedMessage === undefined) { - return undefined; - } - - const [userFullName, userIdRaw] = unformattedMessage.split('|'); - - if (userIdRaw !== undefined) { - const userId = makeUserId(Number.parseInt(userIdRaw, 10)); - return allUsersById.get(userId); - } - - for (const user of allUsersById.values()) { - if (user.full_name === userFullName) { - return user; - } - } +type ImperativeHandle = {| + handleMentionSubscribedCheck(completion: string): Promise, + clearMentionWarnings(): void, +|}; - return undefined; - }; +function MentionWarnings(props: Props, ref) { + const { stream, narrow, auth, allUsersById } = props; - showSubscriptionStatusLoadError = (mentionedUser: UserOrBot) => { - const _ = this.context; + const [unsubscribedMentions, setUnsubscribedMentions] = useState([]); - const alertTitle = _('Couldn’t load information about {fullName}', { - fullName: mentionedUser.full_name, - }); - showToast(alertTitle); - }; + const _ = useContext(TranslationContext); /** - * Check whether the message text entered by the user contains - * an @-mention to a user unsubscribed to the current stream, and if - * so, shows a warning. - * - * This function is expected to be called by `ComposeBox` using a ref - * to this component. + * Tries to parse a user object from an @-mention. * * @param completion The autocomplete option chosend by the user. See JSDoc for AutoCompleteView for details. */ - handleMentionSubscribedCheck = async (completion: string) => { - const { narrow, auth, stream } = this.props; - const { unsubscribedMentions } = this.state; + const getUserFromMention = useCallback( + (completion: string): UserOrBot | void => { + const unformattedMessage = completion.split('**')[1]; + + // We skip user groups, for which autocompletes are of the form + // `**`, and therefore, message.split('**')[1] + // is undefined. + if (unformattedMessage === undefined) { + return undefined; + } - if (is1to1PmNarrow(narrow)) { - return; - } - const mentionedUser = this.getUserFromMention(completion); - if (mentionedUser === undefined || unsubscribedMentions.includes(mentionedUser.user_id)) { - return; - } + const [userFullName, userIdRaw] = unformattedMessage.split('|'); - let isSubscribed: boolean; - try { - isSubscribed = ( - await api.getSubscriptionToStream(auth, mentionedUser.user_id, stream.stream_id) - ).is_subscribed; - } catch (err) { - this.showSubscriptionStatusLoadError(mentionedUser); - return; - } + if (userIdRaw !== undefined) { + const userId = makeUserId(Number.parseInt(userIdRaw, 10)); + return allUsersById.get(userId); + } - if (!isSubscribed) { - this.setState(prevState => ({ - unsubscribedMentions: [...prevState.unsubscribedMentions, mentionedUser.user_id], - })); - } - }; - - handleMentionWarningDismiss = (user: UserOrBot) => { - this.setState(prevState => ({ - unsubscribedMentions: prevState.unsubscribedMentions.filter(x => x !== user.user_id), - })); - }; - - clearMentionWarnings = () => { - this.setState({ - unsubscribedMentions: [], - }); - }; - - render() { - const { unsubscribedMentions } = this.state; - const { stream, narrow, allUsersById } = this.props; - - if (is1to1PmNarrow(narrow)) { - return null; - } + for (const user of allUsersById.values()) { + if (user.full_name === userFullName) { + return user; + } + } - const mentionWarnings = []; - for (const userId of unsubscribedMentions) { - const user = allUsersById.get(userId); + return undefined; + }, + [allUsersById], + ); + + const showSubscriptionStatusLoadError = useCallback( + (mentionedUser: UserOrBot) => { + const alertTitle = _('Couldn’t load information about {fullName}', { + fullName: mentionedUser.full_name, + }); + showToast(alertTitle); + }, + [_], + ); + + useImperativeHandle( + ref, + () => ({ + /** + * Check whether the message text entered by the user contains + * an @-mention to a user unsubscribed to the current stream, and if + * so, shows a warning. + * + * This function is expected to be called by `ComposeBox` using a ref + * to this component. + * + * @param completion The autocomplete option chosend by the user. + See JSDoc for AutoCompleteView for details. + */ + handleMentionSubscribedCheck: async (completion: string) => { + if (is1to1PmNarrow(narrow)) { + return; + } + const mentionedUser = getUserFromMention(completion); + if (mentionedUser === undefined || unsubscribedMentions.includes(mentionedUser.user_id)) { + return; + } + + let isSubscribed: boolean; + try { + isSubscribed = ( + await api.getSubscriptionToStream(auth, mentionedUser.user_id, stream.stream_id) + ).is_subscribed; + } catch (err) { + showSubscriptionStatusLoadError(mentionedUser); + return; + } + + if (!isSubscribed) { + setUnsubscribedMentions(prevUnsubscribedMentions => [ + ...prevUnsubscribedMentions, + mentionedUser.user_id, + ]); + } + }, + + clearMentionWarnings: () => { + setUnsubscribedMentions([]); + }, + }), + [ + auth, + getUserFromMention, + narrow, + showSubscriptionStatusLoadError, + stream, + unsubscribedMentions, + ], + ); + + const handleMentionWarningDismiss = useCallback((user: UserOrBot) => { + setUnsubscribedMentions(prevUnsubscribedMentions => + prevUnsubscribedMentions.filter(x => x !== user.user_id), + ); + }, []); + + if (is1to1PmNarrow(narrow)) { + return null; + } - if (user === undefined) { - continue; - } + const mentionWarnings = []; + for (const userId of unsubscribedMentions) { + const user = allUsersById.get(userId); - mentionWarnings.push( - , - ); + if (user === undefined) { + continue; } - return mentionWarnings; + mentionWarnings.push( + , + ); } + + return mentionWarnings; } // $FlowFixMe[missing-annot]. TODO: Use a type checked connect call. @@ -181,4 +178,4 @@ export default connect( null, null, { forwardRef: true }, -)(MentionWarnings); +)(forwardRef(MentionWarnings)); From 56077f9443b915312b0d1b4e53e37ad00ebd79d0 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Wed, 4 Aug 2021 15:34:35 -0400 Subject: [PATCH 2/5] MentionWarnings [nfc]: Use `useSelector` instead of `connect`. An instance of #4837. See the previous commit for more details, and see https://github.com/zulip/zulip-mobile/pull/4942#discussion_r685518108 for why we do React.createRef>() instead of putting something else for createRef's type parameter. --- src/compose/ComposeBox.js | 13 +------------ src/compose/MentionWarnings.js | 28 +++++++--------------------- 2 files changed, 8 insertions(+), 33 deletions(-) diff --git a/src/compose/ComposeBox.js b/src/compose/ComposeBox.js index 155f9a03768..8843b84d814 100644 --- a/src/compose/ComposeBox.js +++ b/src/compose/ComposeBox.js @@ -138,14 +138,7 @@ class ComposeBox extends PureComponent { messageInputRef = React.createRef<$FlowFixMe>(); topicInputRef = React.createRef<$FlowFixMe>(); - // TODO: Type-check this, once we've adjusted our `react-redux` - // wrapper to do the right thing. It should be - // - // mentionWarnings = React.createRef>() - // - // but we need our `react-redux` wrapper to be aware of - // `{ forwardRef: true }`, since we use that. - mentionWarnings = React.createRef(); + mentionWarnings = React.createRef>(); inputBlurTimeoutId: ?TimeoutID = null; @@ -503,10 +496,6 @@ class ComposeBox extends PureComponent { return ( - {/* - $FlowFixMe[incompatible-use]: - `MentionWarnings` should use a type-checked `connect` - */} , -|}; - type Props = $ReadOnly<{| narrow: Narrow, stream: Subscription | {| ...Stream, in_home_view: boolean |}, - - dispatch: Dispatch, - ...SelectorProps, |}>; type ImperativeHandle = {| @@ -32,7 +24,10 @@ type ImperativeHandle = {| |}; function MentionWarnings(props: Props, ref) { - const { stream, narrow, auth, allUsersById } = props; + const { stream, narrow } = props; + + const auth = useSelector(getAuth); + const allUsersById = useSelector(getAllUsersById); const [unsubscribedMentions, setUnsubscribedMentions] = useState([]); @@ -169,13 +164,4 @@ function MentionWarnings(props: Props, ref) { return mentionWarnings; } -// $FlowFixMe[missing-annot]. TODO: Use a type checked connect call. -export default connect( - state => ({ - auth: getAuth(state), - allUsersById: getAllUsersById(state), - }), - null, - null, - { forwardRef: true }, -)(forwardRef(MentionWarnings)); +export default forwardRef(MentionWarnings); From 11bc13054e63d9d3e8088315c772ca6539be7054 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Wed, 4 Aug 2021 15:38:00 -0400 Subject: [PATCH 3/5] MentionWarnings [nfc]: Move a jsdoc. --- src/compose/MentionWarnings.js | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/compose/MentionWarnings.js b/src/compose/MentionWarnings.js index 76bebbf7075..08462a928f8 100644 --- a/src/compose/MentionWarnings.js +++ b/src/compose/MentionWarnings.js @@ -19,7 +19,19 @@ type Props = $ReadOnly<{| |}>; type ImperativeHandle = {| + /** + * Check whether the message text entered by the user contains + * an @-mention to a user unsubscribed to the current stream, and if + * so, shows a warning. + * + * This function is expected to be called by `ComposeBox` using a ref + * to this component. + * + * @param completion The autocomplete option chosend by the user. + See JSDoc for AutoCompleteView for details. + */ handleMentionSubscribedCheck(completion: string): Promise, + clearMentionWarnings(): void, |}; @@ -81,17 +93,6 @@ function MentionWarnings(props: Props, ref) { useImperativeHandle( ref, () => ({ - /** - * Check whether the message text entered by the user contains - * an @-mention to a user unsubscribed to the current stream, and if - * so, shows a warning. - * - * This function is expected to be called by `ComposeBox` using a ref - * to this component. - * - * @param completion The autocomplete option chosend by the user. - See JSDoc for AutoCompleteView for details. - */ handleMentionSubscribedCheck: async (completion: string) => { if (is1to1PmNarrow(narrow)) { return; From c27c146119cff27222bf1d379934862995a445f3 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Wed, 4 Aug 2021 15:40:36 -0400 Subject: [PATCH 4/5] MentionWarnings [nfc]: Pull out a more generic bit of jsdoc. --- src/compose/MentionWarnings.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/compose/MentionWarnings.js b/src/compose/MentionWarnings.js index 08462a928f8..8e89e2ff79e 100644 --- a/src/compose/MentionWarnings.js +++ b/src/compose/MentionWarnings.js @@ -18,17 +18,18 @@ type Props = $ReadOnly<{| stream: Subscription | {| ...Stream, in_home_view: boolean |}, |}>; +/** + * Functions expected to be called by ComposeBox using a ref to this + * component. + */ type ImperativeHandle = {| /** * Check whether the message text entered by the user contains * an @-mention to a user unsubscribed to the current stream, and if * so, shows a warning. * - * This function is expected to be called by `ComposeBox` using a ref - * to this component. - * * @param completion The autocomplete option chosend by the user. - See JSDoc for AutoCompleteView for details. + * See JSDoc for AutoCompleteView for details. */ handleMentionSubscribedCheck(completion: string): Promise, From e6428f36df78c9f835cc45c74069addc1d5bddd1 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Wed, 4 Aug 2021 16:35:54 -0400 Subject: [PATCH 5/5] MentionWarnings types [nfc]: Annotate default export. This gets us closer to switching to Flow's new "Types-First" mode; that's #4907. --- src/compose/MentionWarnings.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/compose/MentionWarnings.js b/src/compose/MentionWarnings.js index 8e89e2ff79e..b9b44ad4b31 100644 --- a/src/compose/MentionWarnings.js +++ b/src/compose/MentionWarnings.js @@ -1,6 +1,7 @@ /* @flow strict-local */ import React, { useState, useCallback, useContext, forwardRef, useImperativeHandle } from 'react'; +import type { AbstractComponent, Node } from 'react'; import { useSelector } from 'react-redux'; import type { Stream, Narrow, UserOrBot, Subscription, UserId } from '../types'; @@ -36,7 +37,7 @@ type ImperativeHandle = {| clearMentionWarnings(): void, |}; -function MentionWarnings(props: Props, ref) { +function MentionWarningsInner(props: Props, ref): Node { const { stream, narrow } = props; const auth = useSelector(getAuth); @@ -166,4 +167,8 @@ function MentionWarnings(props: Props, ref) { return mentionWarnings; } -export default forwardRef(MentionWarnings); +const MentionWarnings: AbstractComponent = forwardRef( + MentionWarningsInner, +); + +export default MentionWarnings;