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 bug with The close button linked to "Request money. Get $250" is not functioning #36068

Merged
merged 4 commits into from
Feb 7, 2024
Merged

Fix bug with The close button linked to "Request money. Get $250" is not functioning #36068

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2df92a1
Fix bug with The close button linked to "Request money. Get $250" is…
ZhenjaHorbach Feb 7, 2024
a2a2c85
Update useMemo dependencies
ZhenjaHorbach Feb 7, 2024
b65b3a8
Fix prettier issue
ZhenjaHorbach Feb 7, 2024
42db1fc
Fix comments
ZhenjaHorbach Feb 7, 2024
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
18 changes: 2 additions & 16 deletions src/components/OptionsSelector/BaseOptionsSelector.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,6 @@ const propTypes = {
/** Whether referral CTA should be displayed */
shouldShowReferralCTA: PropTypes.bool,

/** A method triggered when the user closes the call to action banner */
onCallToActionClosed: PropTypes.func,
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove the default value too (defaultProps)


/** Referral content type */
referralContentType: PropTypes.string,

Expand All @@ -56,7 +53,6 @@ const propTypes = {
const defaultProps = {
shouldDelayFocus: false,
shouldShowReferralCTA: false,
onCallToActionClosed: () => {},
referralContentType: CONST.REFERRAL_PROGRAM.CONTENT_TYPES.REFER_FRIEND,
safeAreaPaddingBottomStyle: {},
contentContainerStyles: [],
Expand All @@ -72,7 +68,6 @@ class BaseOptionsSelector extends Component {
this.updateFocusedIndex = this.updateFocusedIndex.bind(this);
this.scrollToIndex = this.scrollToIndex.bind(this);
this.selectRow = this.selectRow.bind(this);
this.closeReferralModal = this.closeReferralModal.bind(this);
this.selectFocusedOption = this.selectFocusedOption.bind(this);
this.addToSelection = this.addToSelection.bind(this);
this.updateSearchValue = this.updateSearchValue.bind(this);
Expand All @@ -95,7 +90,6 @@ class BaseOptionsSelector extends Component {
allOptions,
focusedIndex,
shouldDisableRowSelection: false,
shouldShowReferralModal: this.props.shouldShowReferralCTA,
errorMessage: '',
paginationPage: 1,
disableEnterShortCut: false,
Expand Down Expand Up @@ -266,11 +260,6 @@ class BaseOptionsSelector extends Component {
this.props.onChangeText(value);
}

closeReferralModal() {
this.setState((prevState) => ({shouldShowReferralModal: !prevState.shouldShowReferralModal}));
this.props.onCallToActionClosed(this.props.referralContentType);
}

Comment on lines -269 to -273
Copy link
Contributor

Choose a reason for hiding this comment

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

There is a leftover

this.closeReferralModal = this.closeReferralModal.bind(this);

This should be removed

handleFocusIn() {
const activeElement = document.activeElement;
this.setState({
Expand Down Expand Up @@ -653,12 +642,9 @@ class BaseOptionsSelector extends Component {
</>
)}
</View>
{this.props.shouldShowReferralCTA && this.state.shouldShowReferralModal && (
{this.props.shouldShowReferralCTA && (
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove shouldShowReferralCTA prop. Instead use !dismissedReferralBanners(this.props.referralContentType)

Copy link
Contributor Author

@ZhenjaHorbach ZhenjaHorbach Feb 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I'm actually not sure about this
Because we will have to pass dismissedReferralBanners as a parameter
And it turns out that we are changing one thing for another

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually ignore this comment. I think it's better to keep BaseOptionsSelector onyx-free

<View style={[this.props.themeStyles.ph5, this.props.themeStyles.pb5, this.props.themeStyles.flexShrink0]}>
<ReferralProgramCTA
referralContentType={this.props.referralContentType}
onCloseButtonPress={this.closeReferralModal}
/>
<ReferralProgramCTA referralContentType={this.props.referralContentType} />
</View>
)}

Expand Down
2 changes: 1 addition & 1 deletion src/components/ReferralProgramCTA.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ function ReferralProgramCTA({referralContentType, dismissedReferralBanners}: Ref
const theme = useTheme();

const handleDismissCallToAction = () => {
User.dismissReferralBanner(CONST.REFERRAL_PROGRAM.CONTENT_TYPES.REFER_FRIEND);
User.dismissReferralBanner(referralContentType);
};

if (!referralContentType || dismissedReferralBanners[referralContentType]) {
Expand Down
6 changes: 0 additions & 6 deletions src/pages/NewChatPage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import * as OptionsListUtils from '@libs/OptionsListUtils';
import * as ReportUtils from '@libs/ReportUtils';
import variables from '@styles/variables';
import * as Report from '@userActions/Report';
import * as User from '@userActions/User';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import personalDetailsPropType from './personalDetailsPropType';
Expand Down Expand Up @@ -235,10 +234,6 @@ function NewChatPage({betas, isGroupChat, personalDetails, reports, translate, i
updateOptions();
}, [didScreenTransitionEnd, updateOptions]);

const dismissCallToAction = (referralContentType) => {
User.dismissReferralBanner(referralContentType);
};

const {inputCallbackRef} = useAutoFocusInput();

return (
Expand Down Expand Up @@ -276,7 +271,6 @@ function NewChatPage({betas, isGroupChat, personalDetails, reports, translate, i
shouldShowConfirmButton
shouldShowReferralCTA={!dismissedReferralBanners[CONST.REFERRAL_PROGRAM.CONTENT_TYPES.START_CHAT]}
referralContentType={CONST.REFERRAL_PROGRAM.CONTENT_TYPES.START_CHAT}
onCallToActionClosed={dismissCallToAction}
confirmButtonText={selectedOptions.length > 1 ? translate('newChatPage.createGroup') : translate('newChatPage.createChat')}
textInputAlert={isOffline ? `${translate('common.youAppearToBeOffline')} ${translate('search.resultsAreLimited')}` : ''}
onConfirmSelection={createGroup}
Expand Down
16 changes: 3 additions & 13 deletions src/pages/iou/request/MoneyTemporaryForRefactorRequestParticipantsSelector.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import * as DeviceCapabilities from '@libs/DeviceCapabilities';
import * as OptionsListUtils from '@libs/OptionsListUtils';
import * as ReportUtils from '@libs/ReportUtils';
import reportPropTypes from '@pages/reportPropTypes';
import * as User from '@userActions/User';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';

Expand Down Expand Up @@ -88,7 +87,6 @@ function MoneyTemporaryForRefactorRequestParticipantsSelector({
const styles = useThemeStyles();
const [searchTerm, setSearchTerm] = useState('');
const referralContentType = iouType === CONST.IOU.TYPE.SEND ? CONST.REFERRAL_PROGRAM.CONTENT_TYPES.SEND_MONEY : CONST.REFERRAL_PROGRAM.CONTENT_TYPES.MONEY_REQUEST;
const [shouldShowReferralCTA, setShouldShowReferralCTA] = useState(!dismissedReferralBanners[referralContentType]);
const {isOffline} = useNetwork();
const personalDetails = usePersonalDetails();

Expand Down Expand Up @@ -267,20 +265,12 @@ function MoneyTemporaryForRefactorRequestParticipantsSelector({
onFinish();
}, [shouldShowSplitBillErrorMessage, onFinish]);

const closeCallToActionBanner = useCallback(() => {
setShouldShowReferralCTA(false);
User.dismissReferralBanner(referralContentType);
}, [referralContentType]);

const footerContent = useMemo(
() => (
<View>
{shouldShowReferralCTA && (
{!dismissedReferralBanners[referralContentType] && (
<View style={[styles.flexShrink0, !!participants.length && !shouldShowSplitBillErrorMessage && styles.pb5]}>
<ReferralProgramCTA
referralContentType={referralContentType}
onCloseButtonPress={closeCallToActionBanner}
/>
<ReferralProgramCTA referralContentType={referralContentType} />
</View>
)}

Expand All @@ -303,7 +293,7 @@ function MoneyTemporaryForRefactorRequestParticipantsSelector({
)}
</View>
),
[handleConfirmSelection, participants.length, referralContentType, shouldShowSplitBillErrorMessage, shouldShowReferralCTA, styles, translate, closeCallToActionBanner],
[handleConfirmSelection, participants.length, dismissedReferralBanners, referralContentType, shouldShowSplitBillErrorMessage, styles, translate],
);

const itemRightSideComponent = useCallback(
Expand Down
16 changes: 3 additions & 13 deletions src/pages/iou/steps/MoneyRequstParticipantsPage/MoneyRequestParticipantsSelector.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import useThemeStyles from '@hooks/useThemeStyles';
import * as DeviceCapabilities from '@libs/DeviceCapabilities';
import * as OptionsListUtils from '@libs/OptionsListUtils';
import reportPropTypes from '@pages/reportPropTypes';
import * as User from '@userActions/User';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';

Expand Down Expand Up @@ -92,7 +91,6 @@ function MoneyRequestParticipantsSelector({
const styles = useThemeStyles();
const [searchTerm, setSearchTerm] = useState('');
const referralContentType = iouType === CONST.IOU.TYPE.SEND ? CONST.REFERRAL_PROGRAM.CONTENT_TYPES.SEND_MONEY : CONST.REFERRAL_PROGRAM.CONTENT_TYPES.MONEY_REQUEST;
const [shouldShowReferralCTA, setShouldShowReferralCTA] = useState(!dismissedReferralBanners[referralContentType]);
const {isOffline} = useNetwork();
const personalDetails = usePersonalDetails();

Expand Down Expand Up @@ -283,20 +281,12 @@ function MoneyRequestParticipantsSelector({
navigateToSplit();
}, [shouldShowSplitBillErrorMessage, navigateToSplit]);

const closeCallToActionBanner = useCallback(() => {
setShouldShowReferralCTA(false);
User.dismissReferralBanner(referralContentType);
}, [referralContentType]);

const footerContent = useMemo(
() => (
<View>
{shouldShowReferralCTA && (
{!dismissedReferralBanners[referralContentType] && (
<View style={[styles.flexShrink0, !!participants.length && !shouldShowSplitBillErrorMessage && styles.pb5]}>
<ReferralProgramCTA
referralContentType={referralContentType}
onCloseButtonPress={closeCallToActionBanner}
/>
<ReferralProgramCTA referralContentType={referralContentType} />
</View>
)}

Expand All @@ -319,7 +309,7 @@ function MoneyRequestParticipantsSelector({
)}
</View>
),
[handleConfirmSelection, participants.length, referralContentType, shouldShowSplitBillErrorMessage, shouldShowReferralCTA, styles, translate, closeCallToActionBanner],
[handleConfirmSelection, participants.length, dismissedReferralBanners, referralContentType, shouldShowSplitBillErrorMessage, styles, translate],
);

const itemRightSideComponent = useCallback(
Expand Down
Loading