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

Jump to bottom

Added shouldShowOfflineIndicator default props #20467

Merged
merged 17 commits into from
Jun 20, 2023
Merged

Added shouldShowOfflineIndicator default props #20467

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
1504a3e
Added shouldShowOfflineIndicator default props
dhairyasenjaliya Jun 8, 2023
e586701
added props to component to display or not
dhairyasenjaliya Jun 8, 2023
8f6e336
added on plaid step
dhairyasenjaliya Jun 8, 2023
2b9185a
added on personal bank account
dhairyasenjaliya Jun 8, 2023
6936582
added on OnfidoStep
dhairyasenjaliya Jun 8, 2023
b872b57
added on Reimbursement
dhairyasenjaliya Jun 8, 2023
12fc924
added on WalletStatmentPage:
dhairyasenjaliya Jun 8, 2023
89b6568
added on CodesPage
dhairyasenjaliya Jun 8, 2023
6b5e7eb
added on VerifyPage
dhairyasenjaliya Jun 8, 2023
072bbf1
added on SuccesPage
dhairyasenjaliya Jun 8, 2023
4cd8d9c
added on DisablePage
dhairyasenjaliya Jun 8, 2023
73af0c2
Merge branch 'main' of https://github.com/dhairyasenjaliya/Expensify …
dhairyasenjaliya Jun 15, 2023
819ab5a
Added FullPageOfflineBlockingView to plaid
dhairyasenjaliya Jun 15, 2023
b5fc55a
Merge branch 'main' of https://github.com/dhairyasenjaliya/Expensify …
dhairyasenjaliya Jun 16, 2023
07c7103
OfflineIndicator position fix
dhairyasenjaliya Jun 16, 2023
ef63157
OfflineIndicator position bottom with scrollview
dhairyasenjaliya Jun 16, 2023
ede530b
Merge branch 'main' into offlineIndicator
dhairyasenjaliya Jun 19, 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
4 changes: 2 additions & 2 deletions src/components/AddPlaidBankAccount.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ class AddPlaidBankAccount extends React.Component {

// Plaid bank accounts view
return (
<View>
<FullPageOfflineBlockingView>
{!_.isEmpty(this.props.text) && <Text style={[styles.mb5]}>{this.props.text}</Text>}
<View style={[styles.flexRow, styles.alignItemsCenter, styles.mb5]}>
<Icon
Expand All @@ -219,7 +219,7 @@ class AddPlaidBankAccount extends React.Component {
value={this.props.selectedPlaidAccountID}
/>
</View>
</View>
</FullPageOfflineBlockingView>
);
}
}
Expand Down
5 changes: 4 additions & 1 deletion src/components/ReimbursementAccountLoadingIndicator.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ const propTypes = {

function ReimbursementAccountLoadingIndicator(props) {
return (
<ScreenWrapper style={[StyleSheet.absoluteFillObject, styles.reimbursementAccountFullScreenLoading]}>
<ScreenWrapper
shouldShowOfflineIndicator={false}
style={[StyleSheet.absoluteFillObject, styles.reimbursementAccountFullScreenLoading]}
>
<HeaderWithBackButton
title={props.translate('reimbursementAccountLoadingAnimation.oneMoment')}
onBackButtonPress={props.onBackButtonPress}
Expand Down
2 changes: 1 addition & 1 deletion src/components/ScreenWrapper/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ class ScreenWrapper extends React.Component {
})
: this.props.children
}
{this.props.isSmallScreenWidth && <OfflineIndicator />}
{this.props.isSmallScreenWidth && this.props.shouldShowOfflineIndicator && <OfflineIndicator />}
</PickerAvoidingView>
</KeyboardAvoidingView>
</View>
Expand Down
4 changes: 4 additions & 0 deletions src/components/ScreenWrapper/propTypes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ const propTypes = {
...windowDimensionsPropTypes,

...environmentPropTypes,

/** Whether to show offline indicator */
shouldShowOfflineIndicator: PropTypes.bool,
};

const defaultProps = {
Expand All @@ -53,6 +56,7 @@ const defaultProps = {
keyboardAvoidingViewBehavior: 'padding',
shouldEnableMaxHeight: false,
shouldEnablePickerAvoiding: true,
shouldShowOfflineIndicator: true,
};

export {propTypes, defaultProps};
1 change: 1 addition & 0 deletions src/pages/AddPersonalBankAccountPage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ class AddPersonalBankAccountPage extends React.Component {
<ScreenWrapper
includeSafeAreaPaddingBottom={shouldShowSuccess}
shouldEnablePickerAvoiding={false}
shouldShowOfflineIndicator={false}
Copy link
Member

Choose a reason for hiding this comment

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

Looks like we will need the offline indicator here

Screen.Recording.2023-06-09.at.10.00.34.PM.mov

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@thesahindia Even if I remove the props still there is no offline indicator, this bug exists on the main as well un-related to PR
Screenshot 2023-06-09 at 10 54 33 PM

Copy link
Contributor Author

@dhairyasenjaliya dhairyasenjaliya Jun 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@thesahindia I agree we should add <FullPageOfflineBlockingView> to plaid and I found that initially this might be missed to add <FullPageOfflineBlockingView> here AddPlaidBankAccount.js but I believe this should be treated as a separate bug and treated into another PR since this PR is just about to remove the duplicate indicator
let me know if this sounds fine

Changes to add OfflineBlockingView to AddPlaidBankAccount when we have account's

App/src/components/AddPlaidBankAccount.js

Line 200 in d91fe14

<View> 

+ <FullPageOfflineBlockingView>

Copy link
Member

Choose a reason for hiding this comment

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

I am not sure about it. I think we should handle it here. It's a simple change.

Copy link
Member

Choose a reason for hiding this comment

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

cc: @flodnv for thoughts.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes please let's just make this change here @dhairyasenjaliya

Copy link
Member

Choose a reason for hiding this comment

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

Please always add a new comment instead of editing an existing comment, as it can often go unnoticed.

@dhairyasenjaliya, can you please add some more details? I am not sure I understand how this is related to FullPageOfflineBlockingView.

>
<HeaderWithBackButton
title={this.props.translate('bankAccount.addBankAccount')}
Expand Down
1 change: 1 addition & 0 deletions src/pages/ReimbursementAccount/BankAccountPlaidStep.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ class BankAccountPlaidStep extends React.Component {
<ScreenWrapper
includeSafeAreaPaddingBottom={false}
shouldEnablePickerAvoiding={false}
shouldShowOfflineIndicator={false}
>
<HeaderWithBackButton
title={this.props.translate('workspace.common.connectBankAccount')}
Expand Down
5 changes: 4 additions & 1 deletion src/pages/ReimbursementAccount/RequestorOnfidoStep.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,10 @@ class RequestorOnfidoStep extends React.Component {

render() {
return (
<ScreenWrapper includeSafeAreaPaddingBottom={false}>
<ScreenWrapper
includeSafeAreaPaddingBottom={false}
shouldShowOfflineIndicator={false}
>
<HeaderWithBackButton
title={this.props.translate('requestorStep.headerTitle')}
stepCounter={{step: 3, total: 5}}
Expand Down
2 changes: 1 addition & 1 deletion src/pages/settings/Security/TwoFactorAuth/CodesPage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ function CodesPage(props) {
}, []);

return (
<ScreenWrapper>
<ScreenWrapper shouldShowOfflineIndicator={false}>
<HeaderWithBackButton
title={props.translate('twoFactorAuth.headerTitle')}
shouldShowStepCounter
Expand Down
2 changes: 1 addition & 1 deletion src/pages/settings/Security/TwoFactorAuth/DisablePage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ function DisablePage(props) {
}, []);

return (
<ScreenWrapper>
<ScreenWrapper shouldShowOfflineIndicator={false}>
<HeaderWithBackButton
title={props.translate('twoFactorAuth.disableTwoFactorAuth')}
onBackButtonPress={() => Navigation.goBack(ROUTES.SETTINGS_SECURITY)}
Expand Down
2 changes: 1 addition & 1 deletion src/pages/settings/Security/TwoFactorAuth/SuccessPage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const defaultProps = {};

function SuccessPage(props) {
return (
<ScreenWrapper>
<ScreenWrapper shouldShowOfflineIndicator={false}>
<HeaderWithBackButton
title={props.translate('twoFactorAuth.headerTitle')}
shouldShowStepCounter
Expand Down
2 changes: 1 addition & 1 deletion src/pages/settings/Security/TwoFactorAuth/VerifyPage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ function VerifyPage(props) {
}

return (
<ScreenWrapper>
<ScreenWrapper shouldShowOfflineIndicator={false}>
<HeaderWithBackButton
title={props.translate('twoFactorAuth.headerTitle')}
shouldShowStepCounter
Expand Down
5 changes: 4 additions & 1 deletion src/pages/wallet/WalletStatementPage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,10 @@ class WalletStatementPage extends React.Component {
const url = `${CONFIG.EXPENSIFY.EXPENSIFY_URL}statement.php?period=${this.yearMonth}`;

return (
<ScreenWrapper includeSafeAreaPaddingBottom={false}>
<ScreenWrapper
shouldShowOfflineIndicator={false}
includeSafeAreaPaddingBottom={false}
>
<HeaderWithBackButton
title={Str.recapitalize(title)}
shouldShowDownloadButton={!this.props.network.isOffline || this.props.walletStatement.isGenerating}
Expand Down