Skip to content
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.

Sign up for GitHub

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

fix: show large suggestion menu only if there is space #28927

Merged
merged 9 commits into from
Oct 12, 2023
Merged

fix: show large suggestion menu only if there is space #28927

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
4d5e108
fix: show large suggestion menu only if there is space
allroundexperts Oct 5, 2023
54fa3c9
feat: merge with main
allroundexperts Oct 8, 2023
342c592
feat: encapsulate chatItemComposeSecondaryRow styles
allroundexperts Oct 8, 2023
c1e5a56
feat: move the hasEnoughSpaceForLargeSuggestion to a util fn
allroundexperts Oct 9, 2023
477a03e
feat: make the condition in spaceAvailableForLargeSuggestionMenu more…
allroundexperts Oct 9, 2023
d14442a
rename the spaceAvailableForLargeSuggestionMenu fn
allroundexperts Oct 9, 2023
af47c4c
feat: use total suggestions to calculate the menu height
allroundexperts Oct 9, 2023
32d8147
feat: merge with main
allroundexperts Oct 11, 2023
410ae8c
feat: merge with main
allroundexperts Oct 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/CONST.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -878,6 +878,7 @@ const CONST = {
HERE_TEXT: '@here',
},
COMPOSER_MAX_HEIGHT: 125,
CHAT_FOOTER_SECONDARY_ROW_HEIGHT: 25,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This value has to be kept in sync with something, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm... Yes. This is actually the space which is below the composer. It consists of padding and margin as well. That is why I did not use it directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay. Handled what you wanted!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't have any solution in mind yet. Sometimes, a comment like "This has to be kept in sync with..." can work. I'll see what you figured out in a moment.

CHAT_FOOTER_MIN_HEIGHT: 65,
CHAT_SKELETON_VIEW: {
AVERAGE_ROW_HEIGHT: 80,
Expand Down
5 changes: 4 additions & 1 deletion src/pages/home/ReportScreen.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ function ReportScreen({
const prevReport = usePrevious(report);
const prevUserLeavingStatus = usePrevious(userLeavingStatus);
const [isBannerVisible, setIsBannerVisible] = useState(true);
const [listHeight, setListHeight] = useState(0);

const reportID = getReportID(route);
const {addWorkspaceRoomOrChatPendingAction, addWorkspaceRoomOrChatErrors} = ReportUtils.getReportOfflinePendingActionAndErrors(report);
Expand Down Expand Up @@ -343,7 +344,8 @@ function ReportScreen({
}
}, [report, didSubscribeToReportLeavingEvents, reportID]);

const onListLayout = useCallback(() => {
const onListLayout = useCallback((e) => {
setListHeight((prev) => lodashGet(e, 'nativeEvent.layout.height', prev));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this is the most interesting part that wasn't present in the proposal

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. There are some places where we also show a Pay With Expensify button in the header. This causes the height of the header to be different from what it is normally. To cater for this edge case, we need to get header height accurately.

if (!markReadyForHydration) {
return;
}
Expand Down Expand Up @@ -430,6 +432,7 @@ function ReportScreen({
isComposerFullSize={isComposerFullSize}
onSubmitComment={onSubmitComment}
policies={policies}
listHeight={listHeight}
/>
) : (
<ReportFooter shouldDisableCompose />
Expand Down
8 changes: 8 additions & 0 deletions src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ function ComposerWithSuggestions({
submitForm,
shouldShowComposeInput,
measureParentContainer,
listHeight,
// Refs
suggestionsRef,
animatedRef,
Expand Down Expand Up @@ -128,6 +129,12 @@ function ComposerWithSuggestions({
// A flag to indicate whether the onScroll callback is likely triggered by a layout change (caused by text change) or not
const isScrollLikelyLayoutTriggered = useRef(false);

const hasEnoughSpaceForLargeSuggestion =
listHeight - composerHeight - CONST.CHAT_FOOTER_SECONDARY_ROW_HEIGHT >
CONST.AUTO_COMPLETE_SUGGESTER.MAX_AMOUNT_OF_VISIBLE_SUGGESTIONS_IN_CONTAINER * CONST.AUTO_COMPLETE_SUGGESTER.SUGGESTION_ROW_HEIGHT +
CONST.AUTO_COMPLETE_SUGGESTER.SUGGESTER_INNER_PADDING * 2;
const isAutoSuggestionPickerLarge = !isSmallScreenWidth || (isSmallScreenWidth && hasEnoughSpaceForLargeSuggestion);

/**
* Update frequently used emojis list. We debounce this method in the constructor so that UpdateFrequentlyUsedEmojis
* API is not called too often.
Expand Down Expand Up @@ -552,6 +559,7 @@ function ComposerWithSuggestions({
composerHeight={composerHeight}
onInsertedEmoji={onInsertedEmoji}
measureParentContainer={measureParentContainer}
isAutoSuggestionPickerLarge={isAutoSuggestionPickerLarge}
// Input
value={value}
setValue={setValue}
Expand Down
6 changes: 6 additions & 0 deletions src/pages/home/report/ReportActionCompose/ReportActionCompose.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ const propTypes = {
/** Whether user interactions should be disabled */
disabled: PropTypes.bool,

/** Height of the list which the composer is part of */
listHeight: PropTypes.number,

// The NVP describing a user's block status
blockedFromConcierge: PropTypes.shape({
// The date that the user will be unblocked
Expand All @@ -83,6 +86,7 @@ const defaultProps = {
isComposerFullSize: false,
pendingAction: null,
shouldShowComposeInput: true,
listHeight: 0,
...withCurrentUserPersonalDetailsDefaultProps,
};

Expand All @@ -104,6 +108,7 @@ function ReportActionCompose({
report,
reportID,
reportActions,
listHeight,
shouldShowComposeInput,
}) {
const {translate} = useLocalize();
Expand Down Expand Up @@ -386,6 +391,7 @@ function ReportActionCompose({
onFocus={onFocus}
onBlur={onBlur}
measureParentContainer={measureContainer}
listHeight={listHeight}
/>
<ReportDropUI
onDrop={(e) => {
Expand Down
12 changes: 5 additions & 7 deletions src/pages/home/report/ReportActionCompose/Suggestions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import React, {useRef, useCallback, useImperativeHandle} from 'react';
import PropTypes from 'prop-types';
import SuggestionMention from './SuggestionMention';
import SuggestionEmoji from './SuggestionEmoji';
import useWindowDimensions from '../../../../hooks/useWindowDimensions';
import * as SuggestionProps from './suggestionProps';

const propTypes = {
Expand All @@ -15,11 +14,15 @@ const propTypes = {
/** Function to clear the input */
resetKeyboardInput: PropTypes.func.isRequired,

/** Is auto suggestion picker large */
isAutoSuggestionPickerLarge: PropTypes.bool,

...SuggestionProps.baseProps,
};

const defaultProps = {
forwardedRef: null,
isAutoSuggestionPickerLarge: true,
};

/**
Expand All @@ -40,6 +43,7 @@ function Suggestions({
onInsertedEmoji,
resetKeyboardInput,
measureParentContainer,
isAutoSuggestionPickerLarge,
}) {
const suggestionEmojiRef = useRef(null);
const suggestionMentionRef = useRef(null);
Expand Down Expand Up @@ -90,12 +94,6 @@ function Suggestions({
[onSelectionChange, resetSuggestions, setShouldBlockSuggestionCalc, triggerHotkeyActions, updateShouldShowSuggestionMenuToFalse],
);

const {windowHeight, isSmallScreenWidth} = useWindowDimensions();

// the larger composerHeight the less space for EmojiPicker, Pixel 2 has pretty small screen and this value equal 5.3
const hasEnoughSpaceForLargeSuggestion = windowHeight / composerHeight >= 6.8;
const isAutoSuggestionPickerLarge = !isSmallScreenWidth || (isSmallScreenWidth && hasEnoughSpaceForLargeSuggestion);

const baseProps = {
value,
setValue,
Expand Down
5 changes: 5 additions & 0 deletions src/pages/home/report/ReportFooter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ const propTypes = {
/** Whether user interactions should be disabled */
shouldDisableCompose: PropTypes.bool,

/** Height of the list which the composer is part of */
listHeight: PropTypes.number,

...windowDimensionsPropTypes,
};

Expand All @@ -48,6 +51,7 @@ const defaultProps = {
pendingAction: null,
shouldShowComposeInput: true,
shouldDisableCompose: false,
listHeight: 0,
};

function ReportFooter(props) {
Expand Down Expand Up @@ -86,6 +90,7 @@ function ReportFooter(props) {
pendingAction={props.pendingAction}
isComposerFullSize={props.isComposerFullSize}
disabled={props.shouldDisableCompose}
listHeight={props.listHeight}
/>
</SwipeableView>
</View>
Expand Down