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

[HOLD for payment 2024-07-17][$500] Move "Troubleshoot" out of About and into Settings as its own nav item #38594

Closed
shawnborton opened this issue Mar 19, 2024 · 80 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item. Weekly KSv2

Comments

@shawnborton
Copy link
Contributor

shawnborton commented Mar 19, 2024

We want to take the Troubleshoot page:
CleanShot 2024-03-19 at 10 19 04@2x

And move it into the Settings navigation as it's own nav item, right above the Sign Out option at the bottom. When doing this, the Troubleshoot UI should look like the About page, where we use the card style and a nice header with an illustration + h2:
image

cc @TMisiukiewicz since I think you originally implemented this? This would just be a follow up to that.

cc @muttmuure @Expensify/design

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01928d4373947e3fe3
  • Upwork Job ID: 1770771675763638272
  • Last Price Increase: 2024-03-28
  • Automatic offers:
    • alitoshmatov | Reviewer | 0
    • tienifr | Contributor | 0
Issue OwnerCurrent Issue Owner: @muttmuure
@shawnborton shawnborton added the NewFeature Something to build that is a new item. label Mar 19, 2024
Copy link

melvin-bot bot commented Mar 19, 2024

@melvin-bot melvin-bot bot added the Weekly KSv2 label Mar 19, 2024
Copy link

melvin-bot bot commented Mar 19, 2024

⚠️ It looks like this issue is labelled as a New Feature but not tied to any GitHub Project. Keep in mind that all new features should be tied to GitHub Projects in order to properly track external CAP software time ⚠️

@allgandalf
Copy link
Contributor

Can this be opened to external contributors as this is a relatively simple feature to implement :)

@ghost
Copy link

ghost commented Mar 19, 2024

Proposal

Please re-state the problem that we are trying to solve in this issue.

Move "Troubleshoot" out of About and into Settings as its own nav item

What is the root cause of that problem?

This is the a New Feature

The Troubleshoot is inside About Page i.e. here :

{
translationKey: 'initialSettingsPage.aboutPage.troubleshoot',
icon: Expensicons.Lightbulb,
action: waitForNavigate(() => Navigation.navigate(ROUTES.SETTINGS_TROUBLESHOOT)),
},

What changes do you think we should make in order to solve the problem?

We need to move it into InitialPage, just above Signout button here :

{
translationKey: signOutTranslationKey,
icon: Expensicons.Exit,
action: () => {
signOut(false);
},
},

Also, we need to modify the Troubleshoot completely to look like About Us Page
https://github.com/Expensify/App/blob/860cca67cd3ee3309af766dee54b97a14df19556/src/pages/settings/AboutPage/TroubleshootPage.tsx

What alternative solutions did you explore? (Optional)

N/A

Result :

ADDED TEST BRANCH - https://github.com/godofoutcasts94/App/tree/moving-troubleshoot-page

Screenshot 2024-03-21 at 6 05 23 PM

Need Illustration to be added.

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Mar 19, 2024
@ghost
Copy link

ghost commented Mar 19, 2024

Updated Proposal

@ghost
Copy link

ghost commented Mar 19, 2024

Upated Proposal, will add updated screenshot in sometime

@muttmuure
Copy link
Contributor

I think we should actually make this an External issue, since Tomasz is working on a higher value performance project

@muttmuure muttmuure added the External Added to denote the issue can be worked on by a contributor label Mar 21, 2024
@melvin-bot melvin-bot bot changed the title Move "Troubleshoot" out of About and into Settings as its own nav item [$500] Move "Troubleshoot" out of About and into Settings as its own nav item Mar 21, 2024
Copy link

melvin-bot bot commented Mar 21, 2024

Job added to Upwork: https://www.upwork.com/jobs/~01928d4373947e3fe3

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Mar 21, 2024
Copy link

melvin-bot bot commented Mar 21, 2024

Triggered auto assignment to Contributor-plus team member for initial proposal review - @alitoshmatov (External)

@ghost
Copy link

ghost commented Mar 21, 2024

Hey @muttmuure, I have already posted a proposal and I am already working. Should I complete it and post a test branch in the proposal ?

@shubham1206agra
Copy link
Contributor

shubham1206agra commented Mar 21, 2024

Proposal

Please re-state the problem that we are trying to solve in this issue.

Move "Troubleshoot" out of About and into Settings as its own nav item

What is the root cause of that problem?

New Feature request.

What changes do you think we should make in order to solve the problem?

We need to move the Troubleshoot from inside the About Page

{
translationKey: 'initialSettingsPage.aboutPage.troubleshoot',
icon: Expensicons.Lightbulb,
action: waitForNavigate(() => Navigation.navigate(ROUTES.SETTINGS_TROUBLESHOOT)),
},

To InitialPage, just above the Signout button

{
translationKey: signOutTranslationKey,
icon: Expensicons.Exit,
action: () => {
signOut(false);
},
},

We also need to update the CENTRAL_PANE_TO_RHP_MAPPING to

[SCREENS.SETTINGS.ABOUT]: [SCREENS.SETTINGS.APP_DOWNLOAD_LINKS],
[SCREENS.SETTINGS.TROUBLESHOOT]: [SCREENS.SETTINGS.CONSOLE],

We must also update the CentralPaneNavigatorParamList to add SCREENS.SETTINGS.TROUBLESHOOT.

We also need to update TAB_TO_CENTRAL_PANE_MAPPING to add SCREENS.SETTINGS.TROUBLESHOOT.

The Troubleshoot Page design would be similar to AboutPage.

And minor changes to linkingConfig.

Test branch : https://github.com/shubham1206agra/App/tree/test-troubleshoot-pag

Screenshot

Screenshot 2024-03-21 at 6 07 36 PM

What alternative solutions did you explore? (Optional)

N/A

@ShridharGoel
Copy link
Contributor

ShridharGoel commented Mar 21, 2024

Proposal

Please re-state the problem that we are trying to solve in this issue.

What is the root cause of that problem?

New feature.

What changes do you think we should make in order to solve the problem?

Troubleshoot is currently set as RHP.

In linkingConfig/config.ts

Add this under NAVIGATORS.CENTRAL_PANE_NAVIGATOR

[SCREENS.SETTINGS.TROUBLESHOOT]: {
    path: ROUTES.SETTINGS_TROUBLESHOOT,
    exact: true,
},

And remove it from NAVIGATORS.RIGHT_MODAL_NAVIGATOR.

Add this in BaseCentralPaneNavigator.tsx inside settingsScreen

    [SCREENS.SETTINGS.TROUBLESHOOT]: () => require('../../../../../pages/settings/AboutPage/TroubleshootPage').default as React.ComponentType,

const settingsScreens = {
[SCREENS.SETTINGS.WORKSPACES]: () => require('../../../../../pages/workspace/WorkspacesListPage').default as React.ComponentType,
[SCREENS.SETTINGS.PREFERENCES.ROOT]: () => require('../../../../../pages/settings/Preferences/PreferencesPage').default as React.ComponentType,
[SCREENS.SETTINGS.SECURITY]: () => require('../../../../../pages/settings/Security/SecuritySettingsPage').default as React.ComponentType,
[SCREENS.SETTINGS.PROFILE.ROOT]: () => require('../../../../../pages/settings/Profile/ProfilePage').default as React.ComponentType,
[SCREENS.SETTINGS.WALLET.ROOT]: () => require('../../../../../pages/settings/Wallet/WalletPage').default as React.ComponentType,
[SCREENS.SETTINGS.ABOUT]: () => require('../../../../../pages/settings/AboutPage/AboutPage').default as React.ComponentType,
} satisfies Screens;

Remove SCREENS.SETTINGS.TROUBLESHOOT from [SCREENS.SETTINGS.ABOUT] in CENTRAL_PANE_TO_RHP_MAPPING.

Add SCREENS.SETTINGS.TROUBLESHOOT in TAB_TO_CENTRAL_PANE_MAPPING

Add [SCREENS.SETTINGS.TROUBLESHOOT]: undefined; in CentralPaneNavigatorParamList.

Add below in InitialSettingsPage items list

{
    translationKey: 'initialSettingsPage.aboutPage.troubleshoot',
    icon: Expensicons.Lightbulb,
    routeName: ROUTES.SETTINGS_TROUBLESHOOT,
},

And remove it the troubleshoot option from About page.

Example code for the UI (can be polished)

<ScreenWrapper
shouldEnablePickerAvoiding={false}
shouldShowOfflineIndicatorInWideScreen
testID={TroubleshootPage.displayName}
>
<HeaderWithBackButton
    title={translate('initialSettingsPage.aboutPage.troubleshoot')}
    shouldShowBackButton={isSmallScreenWidth}
    onBackButtonPress={() => Navigation.goBack()}
    icon={Expensicons.Lightbulb2}
/>
<ScrollView contentContainerStyle={styles.pt3}>
    <View style={[styles.flex1, isSmallScreenWidth ? styles.workspaceSectionMobile : styles.workspaceSection]}>
        <Section
            title={translate('initialSettingsPage.aboutPage.troubleshoot')}
            isCentralPane
            subtitleMuted
            illustration={LottieAnimations.Desk}
            titleStyles={styles.accountSettingsSectionTitle}
            illustrationBackgroundColor={theme.PAGE_THEMES[SCREENS.SETTINGS.TROUBLESHOOT].backgroundColor}
        >
        <View style={[styles.settingsPageBody, styles.mt5]}>
            <Text style={styles.mb4}>
                <Text>{translate('initialSettingsPage.troubleshoot.description')}</Text>{' '}
                <TextLink
                    style={styles.link}
                    onPress={() => Report.navigateToConciergeChat()}
                >
                    {translate('initialSettingsPage.troubleshoot.submitBug')}
                </TextLink>
            </Text>
        </View>
        <View style={[styles.mr8]}>
            <TestToolRow title="Client side logging">
                <Switch
                    accessibilityLabel="Client side logging"
                    isOn={!!shouldStoreLogs}
                    onToggle={() => (shouldStoreLogs ? Console.disableLoggingAndFlushLogs() : Console.setShouldStoreLogs(true))}
                />
            </TestToolRow>
        </View>
        <MenuItemList
            menuItems={menuItems}
            shouldUseSingleExecution
        />
        {/* Enable additional test features in non-production environments */}
        {!isProduction && (
            <View style={[styles.mr8, styles.mt6]}>
                <TestToolMenu />
            </View>
        )}
        <ConfirmModal
            title={translate('common.areYouSure')}
            isVisible={isConfirmationModalVisible}
            onConfirm={() => {
                setIsConfirmationModalVisible(false);
                Onyx.clear(keysToPreserve).then(() => {
                    App.openApp();
                });
            }}
            onCancel={() => setIsConfirmationModalVisible(false)}
            prompt={translate('initialSettingsPage.troubleshoot.confirmResetDescription')}
            confirmText={translate('initialSettingsPage.troubleshoot.resetAndRefresh')}
            cancelText={translate('common.cancel')}
        />
        </Section>
    </View>
</ScrollView>
</ScreenWrapper>
);
Screenshot 2024-03-21 at 5 53 35 PM

Test branch

https://github.com/ShridharGoel/ExpensifyApp/tree/test-branch-for-38594

@Krishna2323
Copy link
Contributor

Proposal

Please re-state the problem that we are trying to solve in this issue.

Move "Troubleshoot" out of About and into Settings as its own nav item

What is the root cause of that problem?

New feature

What changes do you think we should make in order to solve the problem?

Steps we need to follow:

  1. Move the line below from AboutPage to InitialSettingsPage with routeName.
    translationKey: 'initialSettingsPage.aboutPage.troubleshoot',

    To:
  2. Remove SCREENS.SETTINGS.TROUBLESHOOT from places below:
    [SCREENS.SETTINGS.ABOUT]: [SCREENS.SETTINGS.APP_DOWNLOAD_LINKS, SCREENS.SETTINGS.TROUBLESHOOT],

    [SCREENS.SETTINGS.TROUBLESHOOT]: () => require('../../../pages/settings/AboutPage/TroubleshootPage').default as React.ComponentType,

    [SCREENS.SETTINGS.TROUBLESHOOT]: {
    path: ROUTES.SETTINGS_TROUBLESHOOT,
    exact: true,
  3. Add SCREENS.SETTINGS.TROUBLESHOOT mapping to the places below:
    [SCREENS.SETTINGS.ABOUT]: () => require('../../../../../pages/settings/AboutPage/AboutPage').default as React.ComponentType,

    [SCREENS.SETTINGS.ABOUT]: undefined;

    [SCREENS.SETTINGS.ABOUT]: undefined;
  4. We also need to make styling changes

Result

@ShridharGoel
Copy link
Contributor

Proposal

Updated with new UI.

@ShridharGoel
Copy link
Contributor

Proposal

Updated with link of test branch

@tienifr
Copy link
Contributor

tienifr commented Mar 21, 2024

Proposal

Please re-state the problem that we are trying to solve in this issue.

Move TroubleshootPage into the Settings navigation as it's own nav item, and the Troubleshoot UI should look like the About page.

What is the root cause of that problem?

This is new feature request.

What changes do you think we should make in order to solve the problem?

Small note for reviewer: I was not the first ones to post the proposal, partly because I tried to actually get it working and implementing it locally first, so that I can write a high quality proposals and able to highlight the differences and caveats that we need to take note of (mostly in the 1 part above) instead of rushing to a proposal when I didn't have it actually working and looking like design from my local. If you look into the history you can see I'm the first to have a screenshot of the new page actually working and looking exactly like design, I had to made sure of that before I write the proposal.

  1. Let's craft the new TroubleshootPage UI

Some important notes:

  • We need to add a new renderSubtitle prop to Section, because for trouble shoot page, the subtitle will contain a hyperlink
  • There're many different UI elements in the TroubleShootPage, like the TestToolRow, so it's not similar to the AboutPage as above proposal mentioned
  • We need to supply a Lightbulb illustration as the icon in the Header, currently we don't have that in the app so I temporarily substitute by the black light bulb icon
  1. Move the troubleshoot item here to above the sign out button here

  2. Make sure the navigation works with the TroubleshootPage as an independent page, this part is the same as for the AboutPage

[SCREENS.SETTINGS.TROUBLESHOOT]: () => require('../../../../../pages/settings/AboutPage/TroubleshootPage.tsx').default as React.ComponentType,
[SCREENS.SETTINGS.ABOUT]: [SCREENS.SETTINGS.APP_DOWNLOAD_LINKS],
[SCREENS.SETTINGS.TROUBLESHOOT]: [SCREENS.SETTINGS.CONSOLE],
[SCREENS.SETTINGS.TROUBLESHOOT]: {
    path: ROUTES.SETTINGS_TROUBLESHOOT,
    exact: true,
},
SCREENS.SETTINGS.TROUBLESHOOT,

It looks completely like design now (except for the light bulb part which I explained above):
Screenshot 2024-03-21 at 7 07 38 PM

Here's the full code for the UI part

return (
        <ScreenWrapper
            shouldEnablePickerAvoiding={false}
            shouldShowOfflineIndicatorInWideScreen
            testID={TroubleshootPage.displayName}
        >
            <HeaderWithBackButton
                title={translate('initialSettingsPage.aboutPage.troubleshoot')}
                shouldShowBackButton={isSmallScreenWidth}
                onBackButtonPress={() => Navigation.goBack(ROUTES.SETTINGS)}
                icon={Expensicons.Lightbulb}
            />
            <ScrollView contentContainerStyle={styles.pt3}>
                <View style={[styles.flex1, isSmallScreenWidth ? styles.workspaceSectionMobile : styles.workspaceSection]}>
                    <Section
                        title={translate('initialSettingsPage.aboutPage.troubleshoot')}
                        subtitle={translate('initialSettingsPage.troubleshoot.description')}
                        isCentralPane
                        subtitleMuted
                        illustration={LottieAnimations.Desk}
                        titleStyles={styles.accountSettingsSectionTitle}
                        illustrationBackgroundColor={theme.PAGE_THEMES[SCREENS.SETTINGS.TROUBLESHOOT].backgroundColor}
                        renderSubtitle={() => <Text style={[styles.mt4]}>
                            <Text>{translate('initialSettingsPage.troubleshoot.description')}</Text>{' '}
                            <TextLink
                                style={styles.link}
                                onPress={() => Report.navigateToConciergeChat()}
                            >
                                {translate('initialSettingsPage.troubleshoot.submitBug')}
                            </TextLink>
                        </Text>}
                    >
                        <View style={[styles.flex1, styles.mt5]}>
                        <View style={[styles.mr8]}>
                                <TestToolRow title="Client side logging">
                                    <Switch
                                        accessibilityLabel="Client side logging"
                                        isOn={!!shouldStoreLogs}
                                        onToggle={() => (shouldStoreLogs ? Console.disableLoggingAndFlushLogs() : Console.setShouldStoreLogs(true))}
                                    />
                                </TestToolRow>
                            </View>
                            <MenuItemList
                                menuItems={menuItems}
                                shouldUseSingleExecution
                            />
                            {!isProduction && (
                                <View style={[styles.mr8, styles.mt6]}>
                                    <TestToolMenu />
                                </View>
                            )}
                            <ConfirmModal
                                title={translate('common.areYouSure')}
                                isVisible={isConfirmationModalVisible}
                                onConfirm={() => {
                                    setIsConfirmationModalVisible(false);
                                    Onyx.clear(keysToPreserve).then(() => {
                                        App.openApp();
                                    });
                                }}
                                onCancel={() => setIsConfirmationModalVisible(false)}
                                prompt={translate('initialSettingsPage.troubleshoot.confirmResetDescription')}
                                confirmText={translate('initialSettingsPage.troubleshoot.resetAndRefresh')}
                                cancelText={translate('common.cancel')}
                            />
                        </View>
                    </Section>
                </View>
            </ScrollView>
        </ScreenWrapper>

What alternative solutions did you explore? (Optional)

NA

@ShridharGoel
Copy link
Contributor

Proposal

Updated to polish the UI even more.

@ShridharGoel
Copy link
Contributor

Note that I've also added the needed light bulb in Expensicons.tsx, which can be seen in my test branch.

@tienifr
Copy link
Contributor

tienifr commented Mar 21, 2024

Since this is feature implementation, I'd be happy to provide the full test branch for my proposal upon request (although you can see the full result attached in the proposal from the start) 👍

@shubham1206agra
Copy link
Contributor

Proposal

Updated due to people posting complete diff for no reason (which is discouraged), and this is not a good practice.

@ghost
Copy link

ghost commented Mar 21, 2024

Updated Proposal with test branch since in the initial comment it was asked to be assigned to
Screenshot 2024-03-21 at 6 18 02 PM

@alitoshmatov
Copy link
Contributor

@muttmuure This one is ready for payment

@shawnborton
Copy link
Contributor Author

I found another regression with this one: the Troubleshoot page uses white/light text for the status bar text, and thus you can't see the status bar on iOS:
CleanShot 2024-06-19 at 15 55 01@2x

@tienifr @alitoshmatov can you have a look at that please? Thanks!

@tienifr
Copy link
Contributor

tienifr commented Jun 19, 2024

@shawnborton We will update it to something like below, right?

image

@shawnborton
Copy link
Contributor Author

Exactly, check every other page in Settings and you will see the expected behavior.

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Jun 19, 2024
@tienifr
Copy link
Contributor

tienifr commented Jun 19, 2024

@alitoshmatov PR #44048 to fix #38594 (comment) can be reviewed

@alitoshmatov
Copy link
Contributor

#44048 PR was merged and now on staging

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Weekly KSv2 labels Jul 10, 2024
@melvin-bot melvin-bot bot changed the title [HOLD for payment 2024-06-20] [HOLD for payment 2024-06-18] [$500] Move "Troubleshoot" out of About and into Settings as its own nav item [HOLD for payment 2024-07-17] [HOLD for payment 2024-06-20] [HOLD for payment 2024-06-18] [$500] Move "Troubleshoot" out of About and into Settings as its own nav item Jul 10, 2024
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Jul 10, 2024
Copy link

melvin-bot bot commented Jul 10, 2024

Reviewing label has been removed, please complete the "BugZero Checklist".

Copy link

melvin-bot bot commented Jul 10, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.5-13 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2024-07-17. 🎊

For reference, here are some details about the assignees on this issue:

Copy link

melvin-bot bot commented Jul 10, 2024

BugZero Checklist: The PR adding this new feature has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

  • [@alitoshmatov] Please propose regression test steps to ensure the new feature will work correctly on production in further releases.
  • [@muttmuure] Link the GH issue for creating/updating the regression test once above steps have been agreed upon.

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Weekly KSv2 labels Jul 15, 2024
@melvin-bot melvin-bot bot changed the title [HOLD for payment 2024-07-17] [HOLD for payment 2024-06-20] [HOLD for payment 2024-06-18] [$500] Move "Troubleshoot" out of About and into Settings as its own nav item [HOLD for payment 2024-07-22] [HOLD for payment 2024-07-17] [HOLD for payment 2024-06-20] [HOLD for payment 2024-06-18] [$500] Move "Troubleshoot" out of About and into Settings as its own nav item Jul 15, 2024
Copy link

melvin-bot bot commented Jul 15, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.6-8 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2024-07-22. 🎊

For reference, here are some details about the assignees on this issue:

Copy link

melvin-bot bot commented Jul 15, 2024

BugZero Checklist: The PR adding this new feature has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

  • [@alitoshmatov] Please propose regression test steps to ensure the new feature will work correctly on production in further releases.
  • [@muttmuure] Link the GH issue for creating/updating the regression test once above steps have been agreed upon.

@alitoshmatov
Copy link
Contributor

Regression test is here

@alitoshmatov
Copy link
Contributor

Regression period ends on 17th, pr was merged on 10th

@mallenexpensify mallenexpensify changed the title [HOLD for payment 2024-07-22] [HOLD for payment 2024-07-17] [HOLD for payment 2024-06-20] [HOLD for payment 2024-06-18] [$500] Move "Troubleshoot" out of About and into Settings as its own nav item [HOLD for payment 2024-07-17][$500] Move "Troubleshoot" out of About and into Settings as its own nav item Jul 16, 2024
@melvin-bot melvin-bot bot added the Overdue label Jul 24, 2024
@muttmuure
Copy link
Contributor

$500 - @alitoshmatov
$500 - @tienifr

@melvin-bot melvin-bot bot removed the Overdue label Jul 25, 2024
@muttmuure
Copy link
Contributor

Paid

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item. Weekly KSv2
Projects
None yet
Development

No branches or pull requests

10 participants