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.

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: prevent transparent suggestion list by using portal #27771

Merged
merged 5 commits into from
Sep 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ function BaseAutoCompleteSuggestions(props) {
});

const innerHeight = CONST.AUTO_COMPLETE_SUGGESTER.SUGGESTION_ROW_HEIGHT * props.suggestions.length;
const animatedStyles = useAnimatedStyle(() => StyleUtils.getAutoCompleteSuggestionContainerStyle(rowHeight.value, props.shouldIncludeReportRecipientLocalTimeHeight));
const animatedStyles = useAnimatedStyle(() => StyleUtils.getAutoCompleteSuggestionContainerStyle(rowHeight.value));

useEffect(() => {
rowHeight.value = withTiming(measureHeightOfSuggestionRows(props.suggestions.length, props.isSuggestionPickerLarge), {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,6 @@ const propTypes = {
* When this value is false, the suggester will have a height of 2.5 items. When this value is true, the height can be up to 5 items. */
isSuggestionPickerLarge: PropTypes.bool.isRequired,

/** Show that we should include ReportRecipientLocalTime view height */
shouldIncludeReportRecipientLocalTimeHeight: PropTypes.bool.isRequired,
Copy link
Member

Choose a reason for hiding this comment

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

@s-alves10 could you please explain why this isn't needed anymore?

Copy link
Contributor Author

@s-alves10 s-alves10 Sep 19, 2023

Choose a reason for hiding this comment

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

@rushatgabhane

We use that for calculating the top css here, but in cases using portal, this isn't needed anymore

function getAutoCompleteSuggestionContainerStyle(itemsHeight: number, shouldIncludeReportRecipientLocalTimeHeight: boolean): ViewStyle | CSSProperties {


/** create accessibility label for each item */
accessibilityLabelExtractor: PropTypes.func.isRequired,

Expand Down
17 changes: 8 additions & 9 deletions src/components/AutoCompleteSuggestions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import useWindowDimensions from '../../hooks/useWindowDimensions';
* On the native platform, tapping on auto-complete suggestions will not blur the main input.
*/

function AutoCompleteSuggestions({parentContainerRef, ...props}) {
function AutoCompleteSuggestions({measureParentContainer, ...props}) {
const containerRef = React.useRef(null);
const {windowHeight, windowWidth} = useWindowDimensions();
const [{width, left, bottom}, setContainerState] = React.useState({
Expand All @@ -37,11 +37,11 @@ function AutoCompleteSuggestions({parentContainerRef, ...props}) {
}, []);

React.useEffect(() => {
if (!parentContainerRef || !parentContainerRef.current) {
if (!measureParentContainer) {
return;
}
parentContainerRef.current.measureInWindow((x, y, w) => setContainerState({left: x, bottom: windowHeight - y, width: w}));
}, [parentContainerRef, windowHeight, windowWidth]);
measureParentContainer((x, y, w) => setContainerState({left: x, bottom: windowHeight - y, width: w}));
}, [measureParentContainer, windowHeight, windowWidth]);

const componentToRender = (
<BaseAutoCompleteSuggestions
Expand All @@ -51,11 +51,10 @@ function AutoCompleteSuggestions({parentContainerRef, ...props}) {
/>
);

if (!width) {
return componentToRender;
}

return ReactDOM.createPortal(<View style={StyleUtils.getBaseAutoCompleteSuggestionContainerStyle({left, width, bottom})}>{componentToRender}</View>, document.querySelector('body'));
return (
Boolean(width) &&
ReactDOM.createPortal(<View style={StyleUtils.getBaseAutoCompleteSuggestionContainerStyle({left, width, bottom})}>{componentToRender}</View>, document.querySelector('body'))
);
}

AutoCompleteSuggestions.propTypes = propTypes;
Expand Down
11 changes: 8 additions & 3 deletions src/components/AutoCompleteSuggestions/index.native.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
import React from 'react';
import {Portal} from '@gorhom/portal';
import BaseAutoCompleteSuggestions from './BaseAutoCompleteSuggestions';
import {propTypes} from './autoCompleteSuggestionsPropTypes';

function AutoCompleteSuggestions({parentContainerRef, ...props}) {
// eslint-disable-next-line react/jsx-props-no-spreading
return <BaseAutoCompleteSuggestions {...props} />;
function AutoCompleteSuggestions({measureParentContainer, ...props}) {
return (
<Portal hostName="suggestions">
{/* eslint-disable-next-line react/jsx-props-no-spreading */}
<BaseAutoCompleteSuggestions {...props} />
</Portal>
);
}

AutoCompleteSuggestions.propTypes = propTypes;
Expand Down
4 changes: 0 additions & 4 deletions src/components/EmojiSuggestions.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,6 @@ const propTypes = {
* 2.5 items. When this value is true, the height can be up to 5 items. */
isEmojiPickerLarge: PropTypes.bool.isRequired,

/** Show that we should include ReportRecipientLocalTime view height */
shouldIncludeReportRecipientLocalTimeHeight: PropTypes.bool.isRequired,

/** Stores user's preferred skin tone */
preferredSkinToneIndex: PropTypes.number.isRequired,

Expand Down Expand Up @@ -102,7 +99,6 @@ function EmojiSuggestions(props) {
highlightedSuggestionIndex={props.highlightedEmojiIndex}
onSelect={props.onSelect}
isSuggestionPickerLarge={props.isEmojiPickerLarge}
shouldIncludeReportRecipientLocalTimeHeight={props.shouldIncludeReportRecipientLocalTimeHeight}
accessibilityLabelExtractor={keyExtractor}
measureParentContainer={props.measureParentContainer}
/>
Expand Down
4 changes: 0 additions & 4 deletions src/components/MentionSuggestions.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,6 @@ const propTypes = {
* When this value is false, the suggester will have a height of 2.5 items. When this value is true, the height can be up to 5 items. */
isMentionPickerLarge: PropTypes.bool.isRequired,

/** Show that we should include ReportRecipientLocalTime view height */
shouldIncludeReportRecipientLocalTimeHeight: PropTypes.bool.isRequired,

/** Meaures the parent container's position and dimensions. */
measureParentContainer: PropTypes.func,
};
Expand Down Expand Up @@ -125,7 +122,6 @@ function MentionSuggestions(props) {
highlightedSuggestionIndex={props.highlightedMentionIndex}
onSelect={props.onSelect}
isSuggestionPickerLarge={props.isMentionPickerLarge}
shouldIncludeReportRecipientLocalTimeHeight={props.shouldIncludeReportRecipientLocalTimeHeight}
accessibilityLabelExtractor={keyExtractor}
measureParentContainer={props.measureParentContainer}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ function ComposerWithSuggestions({
setIsFullComposerAvailable,
setIsCommentEmpty,
submitForm,
shouldShowReportRecipientLocalTime,
shouldShowComposeInput,
measureParentContainer,
// Refs
Expand Down Expand Up @@ -503,7 +502,6 @@ function ComposerWithSuggestions({
isComposerFullSize={isComposerFullSize}
updateComment={updateComment}
composerHeight={composerHeight}
shouldShowReportRecipientLocalTime={shouldShowReportRecipientLocalTime}
onInsertedEmoji={onInsertedEmoji}
measureParentContainer={measureParentContainer}
// Input
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import _ from 'underscore';
import lodashGet from 'lodash/get';
import {withOnyx} from 'react-native-onyx';
import {useAnimatedRef} from 'react-native-reanimated';
import {PortalHost} from '@gorhom/portal';
import styles from '../../../../styles/styles';
import ONYXKEYS from '../../../../ONYXKEYS';
import * as Report from '../../../../libs/actions/Report';
Expand Down Expand Up @@ -325,6 +326,7 @@ function ReportActionCompose({
ref={containerRef}
style={[shouldShowReportRecipientLocalTime && !lodashGet(network, 'isOffline') && styles.chatItemComposeWithFirstRow, isComposerFullSize && styles.chatItemFullComposeRow]}
>
<PortalHost name="suggestions" />
<OfflineWithFeedback
pendingAction={pendingAction}
style={isComposerFullSize ? styles.chatItemFullComposeRow : {}}
Expand Down Expand Up @@ -387,7 +389,6 @@ function ReportActionCompose({
setIsFullComposerAvailable={setIsFullComposerAvailable}
setIsCommentEmpty={setIsCommentEmpty}
submitForm={submitForm}
shouldShowReportRecipientLocalTime={shouldShowReportRecipientLocalTime}
shouldShowComposeInput={shouldShowComposeInput}
onFocus={onFocus}
onBlur={onBlur}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ function SuggestionEmoji({
setSelection,
updateComment,
isComposerFullSize,
shouldShowReportRecipientLocalTime,
isAutoSuggestionPickerLarge,
forwardedRef,
resetKeyboardInput,
Expand Down Expand Up @@ -235,7 +234,6 @@ function SuggestionEmoji({
isComposerFullSize={isComposerFullSize}
preferredSkinToneIndex={preferredSkinTone}
isEmojiPickerLarge={isAutoSuggestionPickerLarge}
shouldIncludeReportRecipientLocalTimeHeight={shouldShowReportRecipientLocalTime}
measureParentContainer={measureParentContainer}
/>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ function SuggestionMention({
personalDetails,
updateComment,
composerHeight,
shouldShowReportRecipientLocalTime,
forwardedRef,
isAutoSuggestionPickerLarge,
measureParentContainer,
Expand Down Expand Up @@ -284,7 +283,6 @@ function SuggestionMention({
isComposerFullSize={isComposerFullSize}
isMentionPickerLarge={isAutoSuggestionPickerLarge}
composerHeight={composerHeight}
shouldIncludeReportRecipientLocalTimeHeight={shouldShowReportRecipientLocalTime}
measureParentContainer={measureParentContainer}
/>
);
Expand Down
2 changes: 0 additions & 2 deletions src/pages/home/report/ReportActionCompose/Suggestions.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ function Suggestions({
setSelection,
updateComment,
composerHeight,
shouldShowReportRecipientLocalTime,
forwardedRef,
onInsertedEmoji,
resetKeyboardInput,
Expand Down Expand Up @@ -105,7 +104,6 @@ function Suggestions({
isComposerFullSize,
updateComment,
composerHeight,
shouldShowReportRecipientLocalTime,
isAutoSuggestionPickerLarge,
measureParentContainer,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,6 @@ const propTypes = {
/** A method to call when the form is submitted */
submitForm: PropTypes.func.isRequired,

/** Whether the recipient local time is shown or not */
shouldShowReportRecipientLocalTime: PropTypes.bool.isRequired,

/** Whether the compose input is shown or not */
shouldShowComposeInput: PropTypes.bool.isRequired,

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,6 @@ const baseProps = {
/** Callback to update the comment draft */
updateComment: PropTypes.func.isRequired,

/** Flag whether we need to consider the participants */
shouldShowReportRecipientLocalTime: PropTypes.bool.isRequired,

/** Meaures the parent container's position and dimensions. */
measureParentContainer: PropTypes.func.isRequired,
};
Expand Down
6 changes: 2 additions & 4 deletions src/styles/StyleUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -935,19 +935,17 @@ function getBaseAutoCompleteSuggestionContainerStyle({left, bottom, width}: {lef
/**
* Gets the correct position for auto complete suggestion container
*/
function getAutoCompleteSuggestionContainerStyle(itemsHeight: number, shouldIncludeReportRecipientLocalTimeHeight: boolean): ViewStyle | CSSProperties {
function getAutoCompleteSuggestionContainerStyle(itemsHeight: number): ViewStyle | CSSProperties {
'worklet';

const optionalPadding = shouldIncludeReportRecipientLocalTimeHeight ? CONST.RECIPIENT_LOCAL_TIME_HEIGHT : 0;
const padding = CONST.AUTO_COMPLETE_SUGGESTER.SUGGESTER_PADDING + optionalPadding;
const borderWidth = 2;
const height = itemsHeight + 2 * CONST.AUTO_COMPLETE_SUGGESTER.SUGGESTER_INNER_PADDING + borderWidth;

// The suggester is positioned absolutely within the component that includes the input and RecipientLocalTime view (for non-expanded mode only). To position it correctly,
// we need to shift it by the suggester's height plus its padding and, if applicable, the height of the RecipientLocalTime view.
return {
overflow: 'hidden',
top: -(height + padding),
top: -(height + CONST.AUTO_COMPLETE_SUGGESTER.SUGGESTER_PADDING),
height,
};
}
Expand Down
Loading