Skip to content

Commit ebcbeca

Browse files
chrisbobbegnprice
authored andcommitted
ComposeBox: Move compose buttons into a row below message-text input
Making `multiline` constantly true fixes various urgent bugs where the message-text input would lose its contents or fail to update when you press one of these menu buttons on iOS. Details in the linked issues. Fixes: #5080 Fixes: #5291 Fixes: #5463 Fixes: #4628
1 parent 9357b0d commit ebcbeca

File tree

2 files changed

+20
-95
lines changed

2 files changed

+20
-95
lines changed

src/compose/ComposeBox.js

Lines changed: 10 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -199,10 +199,6 @@ const ComposeBox: React$AbstractComponent<Props, ImperativeHandle> = forwardRef(
199199

200200
const inputBlurTimeoutId = useRef<?TimeoutID>(null);
201201

202-
// TODO(#5141): Encapsulate this in a nice, plain action-sheet pattern
203-
// instead of setting it all over the place
204-
const [isMenuExpanded, setIsMenuExpanded] = useState<boolean>(false);
205-
206202
const [height, setHeight] = useState<number>(20);
207203

208204
const [focusState, setFocusState] = useState<{|
@@ -258,20 +254,9 @@ const ComposeBox: React$AbstractComponent<Props, ImperativeHandle> = forwardRef(
258254
if (!isEditing) {
259255
dispatch(draftUpdate(narrow, messageInputValue));
260256
}
261-
setIsMenuExpanded(false);
262257
}
263258
}, [dispatch, isEditing, narrow, messageInputState, prevMessageInputState]);
264259

265-
const prevTopicInputState = usePrevious(topicInputState);
266-
useEffect(() => {
267-
const topicInputValue = topicInputState.value;
268-
const prevTopicInputValue = prevTopicInputState?.value;
269-
270-
if (prevTopicInputValue !== topicInputValue) {
271-
setIsMenuExpanded(false);
272-
}
273-
}, [topicInputState, prevTopicInputState]);
274-
275260
const updateIsFocused = useCallback(() => {
276261
setFocusState(state => ({ ...state, either: state.message || state.topic }));
277262
}, []);
@@ -454,10 +439,6 @@ const ComposeBox: React$AbstractComponent<Props, ImperativeHandle> = forwardRef(
454439
);
455440
useImperativeHandle(ref, () => ({ doQuoteAndReply }), [doQuoteAndReply]);
456441

457-
const handleComposeMenuToggle = useCallback(() => {
458-
setIsMenuExpanded(x => !x);
459-
}, []);
460-
461442
const handleLayoutChange = useCallback((event: LayoutEvent) => {
462443
setHeight(event.nativeEvent.layout.height);
463444
}, []);
@@ -495,13 +476,11 @@ const ComposeBox: React$AbstractComponent<Props, ImperativeHandle> = forwardRef(
495476
topicInputRef.current?.focus();
496477
} else {
497478
setFocusState(state => ({ ...state, message: true, either: true }));
498-
setIsMenuExpanded(false);
499479
}
500480
}, [isEditing, narrow, focusState.either, topicInputState.value, topicInputRef]);
501481

502482
const handleMessageBlur = useCallback(() => {
503483
setFocusState(state => ({ ...state, message: false }));
504-
setIsMenuExpanded(false);
505484
dispatch(sendTypingStop(narrow));
506485
// give a chance to the topic input to get the focus
507486
clearTimeout(inputBlurTimeoutId.current);
@@ -510,21 +489,15 @@ const ComposeBox: React$AbstractComponent<Props, ImperativeHandle> = forwardRef(
510489

511490
const handleTopicFocus = useCallback(() => {
512491
setFocusState(state => ({ ...state, topic: true, either: true }));
513-
setIsMenuExpanded(false);
514492
}, []);
515493

516494
const handleTopicBlur = useCallback(() => {
517495
setFocusState(state => ({ ...state, topic: false }));
518-
setIsMenuExpanded(false);
519496
// give a chance to the message input to get the focus
520497
clearTimeout(inputBlurTimeoutId.current);
521498
inputBlurTimeoutId.current = setTimeout(updateIsFocused, FOCUS_DEBOUNCE_TIME_MS);
522499
}, [updateIsFocused]);
523500

524-
const handleInputTouchStart = useCallback(() => {
525-
setIsMenuExpanded(false);
526-
}, []);
527-
528501
const destinationNarrow = useMemo(() => {
529502
if (isStreamNarrow(narrow) || (isTopicNarrow(narrow) && isEditing)) {
530503
const streamId = streamIdOfNarrow(narrow);
@@ -652,7 +625,8 @@ const ComposeBox: React$AbstractComponent<Props, ImperativeHandle> = forwardRef(
652625
},
653626
composeInputs: {
654627
flex: 1,
655-
paddingVertical: 8,
628+
paddingTop: 8,
629+
paddingLeft: 8,
656630
},
657631
submitButtonContainer: {
658632
padding: 8,
@@ -739,15 +713,6 @@ const ComposeBox: React$AbstractComponent<Props, ImperativeHandle> = forwardRef(
739713
style={styles.composeBox}
740714
onLayout={handleLayoutChange}
741715
>
742-
<ComposeMenu
743-
destinationNarrow={destinationNarrow}
744-
expanded={isMenuExpanded}
745-
insertAttachment={insertAttachment}
746-
insertVideoCallLink={
747-
videoChatProvider !== null ? () => insertVideoCallLink(videoChatProvider) : null
748-
}
749-
onExpandContract={handleComposeMenuToggle}
750-
/>
751716
<View style={styles.composeInputs}>
752717
<Input
753718
style={styles.topicInput}
@@ -761,14 +726,12 @@ const ComposeBox: React$AbstractComponent<Props, ImperativeHandle> = forwardRef(
761726
{...topicInputCallbacks}
762727
onFocus={handleTopicFocus}
763728
onBlur={handleTopicBlur}
764-
onTouchStart={handleInputTouchStart}
765729
onSubmitEditing={() => messageInputRef.current?.focus()}
766730
blurOnSubmit={false}
767731
returnKeyType="next"
768732
/>
769733
<Input
770-
// TODO(#5291): Don't switch between true/false for multiline
771-
multiline={!isMenuExpanded}
734+
multiline
772735
style={styles.composeTextInput}
773736
underlineColorAndroid="transparent"
774737
placeholder={placeholder}
@@ -778,7 +741,13 @@ const ComposeBox: React$AbstractComponent<Props, ImperativeHandle> = forwardRef(
778741
{...messageInputCallbacks}
779742
onBlur={handleMessageBlur}
780743
onFocus={handleMessageFocus}
781-
onTouchStart={handleInputTouchStart}
744+
/>
745+
<ComposeMenu
746+
destinationNarrow={destinationNarrow}
747+
insertAttachment={insertAttachment}
748+
insertVideoCallLink={
749+
videoChatProvider !== null ? () => insertVideoCallLink(videoChatProvider) : null
750+
}
782751
/>
783752
</View>
784753
<View style={styles.submitButtonContainer}>

src/compose/ComposeMenu.js

Lines changed: 10 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -11,25 +11,15 @@ import { TranslationContext } from '../boot/TranslationProvider';
1111
import type { Narrow } from '../types';
1212
import { showErrorAlert } from '../utils/info';
1313
import { BRAND_COLOR, HIGHLIGHT_COLOR, createStyleSheet } from '../styles';
14-
import {
15-
IconPlusCircle,
16-
IconLeft,
17-
IconImage,
18-
IconCamera,
19-
IconAttach,
20-
IconVideo,
21-
} from '../common/Icons';
22-
import AnimatedComponent from '../animation/AnimatedComponent';
14+
import { IconImage, IconCamera, IconAttach, IconVideo } from '../common/Icons';
2315
import { uploadFile } from '../actions';
2416
import { androidEnsureStoragePermission } from '../lightbox/download';
2517
import type { SpecificIconType } from '../common/Icons';
2618

2719
type Props = $ReadOnly<{|
28-
expanded: boolean,
2920
destinationNarrow: Narrow,
3021
insertAttachment: ($ReadOnlyArray<DocumentPickerResponse>) => Promise<void>,
3122
insertVideoCallLink: (() => void) | null,
32-
onExpandContract: () => void,
3323
|}>;
3424

3525
/**
@@ -82,7 +72,7 @@ type MenuButtonProps = $ReadOnly<{|
8272

8373
function MenuButton(props: MenuButtonProps) {
8474
const { onPress, IconComponent } = props;
85-
const style = useMemo(() => ({ padding: 12 }), []);
75+
const style = useMemo(() => ({ paddingHorizontal: 12, paddingVertical: 8 }), []);
8676

8777
return (
8878
<Pressable style={style} onPress={onPress}>
@@ -92,8 +82,7 @@ function MenuButton(props: MenuButtonProps) {
9282
}
9383

9484
export default function ComposeMenu(props: Props): Node {
95-
const { destinationNarrow, insertAttachment, expanded, insertVideoCallLink, onExpandContract } =
96-
props;
85+
const { destinationNarrow, insertAttachment, insertVideoCallLink } = props;
9786

9887
const dispatch = useDispatch();
9988
const _ = useContext(TranslationContext);
@@ -232,53 +221,20 @@ export default function ComposeMenu(props: Props): Node {
232221
container: {
233222
flexDirection: 'row',
234223
},
235-
composeMenu: {
236-
flexDirection: 'row',
237-
238-
// Hide the buttons when the menu is collapsed. (overflowX would
239-
// be great here if RN offered it.)
240-
overflow: 'hidden',
241-
},
242-
expandButton: {
243-
padding: 12,
244-
},
245224
}),
246225
[],
247226
);
248227

249-
const numIcons = 2 + (Platform.OS === 'android' ? 1 : 0) + (insertVideoCallLink !== null ? 1 : 0);
250-
251228
return (
252229
<View style={styles.container}>
253-
<AnimatedComponent
254-
stylePropertyName="width"
255-
fullValue={48 * numIcons}
256-
useNativeDriver={false}
257-
visible={expanded}
258-
>
259-
<View style={styles.composeMenu}>
260-
{Platform.OS === 'android' && (
261-
<MenuButton onPress={handleFilesPicker} IconComponent={IconAttach} />
262-
)}
263-
<MenuButton onPress={handleImagePicker} IconComponent={IconImage} />
264-
<MenuButton onPress={handleCameraCapture} IconComponent={IconCamera} />
265-
{insertVideoCallLink !== null ? (
266-
<MenuButton onPress={insertVideoCallLink} IconComponent={IconVideo} />
267-
) : null}
268-
</View>
269-
</AnimatedComponent>
270-
{!expanded && (
271-
<Pressable style={styles.expandButton} onPress={onExpandContract}>
272-
{({ pressed }) => (
273-
<IconPlusCircle color={pressed ? HIGHLIGHT_COLOR : BRAND_COLOR} size={24} />
274-
)}
275-
</Pressable>
276-
)}
277-
{expanded && (
278-
<Pressable style={styles.expandButton} onPress={onExpandContract}>
279-
{({ pressed }) => <IconLeft color={pressed ? HIGHLIGHT_COLOR : BRAND_COLOR} size={24} />}
280-
</Pressable>
230+
{Platform.OS === 'android' && (
231+
<MenuButton onPress={handleFilesPicker} IconComponent={IconAttach} />
281232
)}
233+
<MenuButton onPress={handleImagePicker} IconComponent={IconImage} />
234+
<MenuButton onPress={handleCameraCapture} IconComponent={IconCamera} />
235+
{insertVideoCallLink !== null ? (
236+
<MenuButton onPress={insertVideoCallLink} IconComponent={IconVideo} />
237+
) : null}
282238
</View>
283239
);
284240
}

0 commit comments

Comments
 (0)