Changeset View
Standalone View
web/sidebar/community-drawer-item.react.js
| // @flow | // @flow | |||||||||||
| import classnames from 'classnames'; | import classnames from 'classnames'; | |||||||||||
| import * as React from 'react'; | import * as React from 'react'; | |||||||||||
| import { useSelector } from 'react-redux'; | import { useSelector } from 'react-redux'; | |||||||||||
| import type { CommunityDrawerItemData } from 'lib/utils/drawer-utils.react'; | import type { CommunityDrawerItemData } from 'lib/utils/drawer-utils.react'; | |||||||||||
| import { | import { useThreadIsActive } from '../selectors/thread-selectors'; | |||||||||||
| useOnClickThread, | import { getCommunityDrawerItemHandler } from './community-drawer-item-handlers.react'; | |||||||||||
| useThreadIsActive, | ||||||||||||
| } from '../selectors/thread-selectors'; | ||||||||||||
| import css from './community-drawer-item.css'; | import css from './community-drawer-item.css'; | |||||||||||
| import { ExpandButton } from './expand-buttons.react'; | import { ExpandButton } from './expand-buttons.react'; | |||||||||||
| import SubchannelsButton from './subchannels-button.react'; | import SubchannelsButton from './subchannels-button.react'; | |||||||||||
| export type DrawerItemProps = { | export type DrawerItemProps = { | |||||||||||
| +itemData: CommunityDrawerItemData<string>, | +itemData: CommunityDrawerItemData<string>, | |||||||||||
| +toggleExpanded: (threadID: string) => void, | +toggleExpanded: (threadID: string) => void, | |||||||||||
| +expanded: boolean, | +expanded: boolean, | |||||||||||
| ▲ Show 20 Lines • Show All 46 Lines • ▼ Show 20 Lines | return ( | |||||||||||
| </div> | </div> | |||||||||||
| ); | ); | |||||||||||
| }, [itemChildren?.length, hasSubchannelsButton, onExpandToggled, expanded]); | }, [itemChildren?.length, hasSubchannelsButton, onExpandToggled, expanded]); | |||||||||||
| const active = useThreadIsActive(threadInfo.id); | const active = useThreadIsActive(threadInfo.id); | |||||||||||
| const isCreateMode = useSelector( | const isCreateMode = useSelector( | |||||||||||
| state => state.navInfo.chatMode === 'create', | state => state.navInfo.chatMode === 'create', | |||||||||||
| ); | ); | |||||||||||
| const onClick = useOnClickThread(threadInfo); | const tab = useSelector(state => state.navInfo.tab); | |||||||||||
| const selectItemIfNotActiveCreation = React.useCallback( | const Handler = React.useMemo(() => getCommunityDrawerItemHandler(tab), [ | |||||||||||
| tab, | ||||||||||||
| ]); | ||||||||||||
tomek: We don't need to memoize the `Handler` because it can only return a couple of const values… | ||||||||||||
| const [handler, setHandler] = React.useState({ | ||||||||||||
| // eslint-disable-next-line no-unused-vars | ||||||||||||
| onClick: event => {}, | ||||||||||||
tomekUnsubmitted Not Done Inline Actions
Why do we need to silence this message? Would removing a parameter solve the issue? tomek: Why do we need to silence this message? Would removing a parameter solve the issue? | ||||||||||||
inkaAuthorUnsubmitted Done Inline ActionsIf I do that Flow doesn't allow me to use this function in line 86, since the function does not expect an argument and i'm passing it an event inka: If I do that Flow doesn't allow me to use this function in line 86, since the function does not… | ||||||||||||
| }); | ||||||||||||
| const onClick = React.useCallback( | ||||||||||||
| (event: SyntheticEvent<HTMLAnchorElement>) => { | (event: SyntheticEvent<HTMLAnchorElement>) => { | |||||||||||
| if (!isCreateMode || !active) { | if (!isCreateMode || !active) { | |||||||||||
tomekUnsubmitted Not Done Inline ActionsAre we sure that these conditions remain correct for all the other apps? What happens when we are creating a chat and then decide to open a calendar? Will the drawer be useful? tomek: Are we sure that these conditions remain correct for all the other apps? What happens when we… | ||||||||||||
inkaAuthorUnsubmitted Done Inline ActionsI started to think about this, and actually here is what happens for ChatThreadList, where I took this code from: I suppose this is a bug? Anyway this condition means that we cannot navigate to a thread if it is both active and in create mode. inka: I started to think about this, and actually here is what happens for `ChatThreadList`, where I… | ||||||||||||
tomekUnsubmitted Not Done Inline ActionsYes, it is a bug - could you create a task for it? Regarding the additional dispatch, if it doesn't affect the history, it doesn't matter that much. But it might be a good idea to make the current thread unclickable. tomek: Yes, it is a bug - could you create a task for it?
Regarding the additional dispatch, if it… | ||||||||||||
inkaAuthorUnsubmitted Done Inline Actions[[ https://linear.app/comm/issue/ENG-2824/[web]-changing-the-tab-while-creating-a-new-chat-results-in-weird | issue ]], (I think the [web] in url is breaking the formatting on phab) inka: [[ https://linear.app/comm/issue/ENG-2824/[web]-changing-the-tab-while-creating-a-new-chat… | ||||||||||||
| onClick(event); | handler.onClick(event); | |||||||||||
| } | } | |||||||||||
| }, | }, | |||||||||||
| [isCreateMode, active, onClick], | [isCreateMode, active, handler], | |||||||||||
| ); | ); | |||||||||||
| const titleLabel = classnames(css.title, css[labelStyle]); | const titleLabel = classnames(css.title, css[labelStyle]); | |||||||||||
| return ( | return ( | |||||||||||
| <> | <> | |||||||||||
| <Handler setHandler={setHandler} threadInfo={threadInfo} /> | ||||||||||||
| <div className={css.threadEntry}> | <div className={css.threadEntry}> | |||||||||||
| {itemExpandButton} | {itemExpandButton} | |||||||||||
| <a onClick={selectItemIfNotActiveCreation} className={css.titleWrapper}> | <a onClick={onClick} className={css.titleWrapper}> | |||||||||||
| <div className={titleLabel}>{threadInfo.uiName}</div> | <div className={titleLabel}>{threadInfo.uiName}</div> | |||||||||||
| </a> | </a> | |||||||||||
| </div> | </div> | |||||||||||
| <div className={css.threadListContainer}>{children}</div> | <div className={css.threadListContainer}>{children}</div> | |||||||||||
| </> | </> | |||||||||||
| ); | ); | |||||||||||
| } | } | |||||||||||
| Show All 33 Lines | ||||||||||||
We don't need to memoize the Handler because it can only return a couple of const values which aren't created inside getCommunityDrawerItemHandler. But this is an implementation detail of getCommunityDrawerItemHandler and relying on that has a tiny chance of causing issues in the future.
A better idea might be to call this function as a part of the selector. This has an additional benefit of a possible performance optimization because there are multiple tabs for which the same handler value is provided.