[HOLD for payment 2024-10-30] [$1000] Create a useResponsiveLayout hook

[roryabraham]

Slack context: https://expensify.slack.com/archives/C01GTK53T8Q/p1698364514493649

Problem

There are some times when we want to use the same layout on mobile devices or in the RHP on wide screens. In these cases, we can't rely on isSmallScreenWidth from useWindowDimensions to determine which layout to use, so we have hard-coded and/or different solutions we use to address the same problem.

Solution

1. In the navigation stack, add an isInRHP route prop to all screens in the RHP.

2. Write a custom hook called useResponsiveLayout that does something like this:

   function useResponsiveLayout() {
       const {isSmallScreenWidth} = useWindowDimensions();
       const {params} = useRoute();
       return {
           shouldUseNarrowLayout = isSmallScreenWidth || params.isInRHP,
       };
   }

3. Replace most layout-motivated uses of useWindowDimensions with useResponsiveLayout like so:

   - const {isSmallScreenWidth} = useWindowDimensions();
   + const {shouldUseNarrowLayout} = useResponsiveLayout();

4. Introduce a new lint rule that prohibits using isSmallScreenWidth from useWindowDimensions, advising developers to use shouldUseNarrowLayout instead.

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01f36dcb081cdf74b2
• Upwork Job ID: 1717949899365408768
• Last Price Increase: 2023-12-29

Issue Owner

Current Issue Owner: @alexpensify

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01f36dcb081cdf74b2

[melvin-bot]

Triggered auto assignment to @maddylewis (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details.

[melvin-bot]

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

[alitoshmatov]

I can work on this if no one is assigned yet, or should I write proposal

Since it is a new feature main idea is explained well in description, and I think anything else could be discussed in the PR

[ZhenjaHorbach]

Proposal

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

Create a useResponsiveLayout hook

What is the root cause of that problem?

This is a new functionality

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

First, we need open RHP modals navigator
And add new route option isInRHP in screenOptions

Then we need to create a new hook
For example, using the suggested option

And then we need to find isSmallScreenWidth values
Remove those values and use shouldUseNarrowLayout from the new hook

And at the last, we need to introduce a new lint rule that disallow using isSmallScreenWidth from useWindowDimensions

We can write a new rule

module.exports = {
  meta: {
    type: 'problem',
    docs: {
      description: 'Do not use isSmallScreenWidth from useWindowDimensions. Instead of that use shouldUseNarrowLayout from useResponsiveLayout',
      category: 'Best Practices',
    },
  },
  create: function (context) {
    return {
      VariableDeclarator(node) {
        if ( node.init.callee && node.init.callee.name === 'useWindowDimensions') {
          const properties = node.id.properties;
          for (const property of properties) {
            if (property.key.name === 'isSmallScreenWidth') {
              context.report({
                node,
                message: 'Do not use isSmallScreenWidth from useWindowDimensions. Instead of that use shouldUseNarrowLayout from useResponsiveLayout',
              });
            }
          }
        }
      },
    };
  },
};

What alternative solutions did you explore? (Optional)

NA

[rayane-djouah]

Proposal

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

The problem we aim to address is the lack of a consistent and reliable method to determine the appropriate layout for mobile devices and wide screens within our application. Currently, we rely on the isSmallScreenWidth property from the useWindowDimensions hook to make layout decisions. However, this approach falls short in situations where we need to maintain the same layout for both mobile devices and the Right-Hand Panel (RHP) on wide screens. As a result, we resort to hard-coded solutions or utilize different methods, which leads to code inconsistency and maintainability challenges.

What is the root cause of that problem?

The root cause of this problem lies in the absence of a dedicated mechanism to determine the layout based on specific conditions, such as whether a screen is within the RHP or on a small screen.

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

1. Introduce isInRHP:

App/src/libs/Navigation/AppNavigator/ModalStackNavigators.tsx

Lines 60 to 64 in 7b836cf

	<ModalStackNavigator.Screen
	key={name}
	name={name}
	getComponent={(screens as Required<Screens>)[name as Screen]}
	/>

Include an isInRHP option with a default value (true).
This initial param will allow screens to determine whether they are within the Right-Hand Panel (RHP) and make layout decisions accordingly.

<ModalStackNavigator.Screen
    key={name}
    name={name}
    getComponent={(screens as Required<Screens>)[name as Screen]}
    initialParams={{isInRHP: true} as TStackParams[string]}  //here
/>

3. Create useResponsiveLayout Custom Hook: Develop a custom hook called useResponsiveLayout that provides a unified way to determine the appropriate layout. This hook will take advantage of useWindowDimensions and the isInRHP route prop to decide whether a narrow layout should be used. The hook's implementation is as follows:

export default function useResponsiveLayout(): ResponsiveLayoutResult {
    const {isSmallScreenWidth} = useWindowDimensions();
    try {
        const {params} = useRoute<RouteProp<RouteParams, 'params'>>();
        return {shouldUseNarrowLayout: isSmallScreenWidth || (params?.isInRHP ?? false)};
    } catch (error) {
        return {
            shouldUseNarrowLayout: isSmallScreenWidth,
        };
    }
}

4. Replace useWindowDimensions with useResponsiveLayout: Update the codebase to replace most instances where useWindowDimensions is used for layout decisions with useResponsiveLayout as follows:

Before:

const { isSmallScreenWidth } = useWindowDimensions();

After:

const { shouldUseNarrowLayout } = useResponsiveLayout();

5. Introduce a Lint Rule: Implement a new linting rule in https://github.com/Expensify/eslint-config-expensify/tree/main/eslint-plugin-expensify that prohibits the direct use of isSmallScreenWidth from useWindowDimensions for layout determination. This rule will encourage developers to use 'shouldUseNarrowLayout' from useResponsiveLayout instead, ensuring consistency in layout decisions throughout the codebase.

const _ = require('underscore');
const message = require('./CONST').MESSAGE.PREFER_USE_RESPONSIVE_FOR_LAYOUT;

module.exports = {
    meta: {
        type: 'problem',
        docs: {
            description: 'Prohibit the direct use of isSmallScreenWidth from useWindowDimensions',
        },
    },
    create(context) {
        return {
            VariableDeclarator(node) {
                if (!node.init || !node.init.callee || node.init.callee.name !== 'useWindowDimensions') {
                    return;
                }
                // Check for 'const {isSmallScreenWidth, ...} = useWindowDimensions();' pattern
                if (node.id.type === 'ObjectPattern') {
                    node.id.properties.forEach((property) => {
                        if (!property.key || property.key.name !== 'isSmallScreenWidth') {
                            return;
                        }
                        context.report({
                            node: property,
                            message,
                        });
                    });
                }

                // Check for 'const var = useWindowDimensions();' and use of this var
                const variableName = node.id.name;
                const variableUsages = _.filter(context.getScope().references, reference => reference.identifier.name === variableName);
                variableUsages.forEach((usage) => {
                    const parent = usage.identifier.parent;

                    // Check for 'const isSmallScreen = var.isSmallScreenWidth;' pattern
                    if (parent.type === 'MemberExpression' && parent.property.name === 'isSmallScreenWidth') {
                        context.report({
                            node: parent.property,
                            message,
                        });
                    }

                    // Check for 'const {isSmallScreenWidth} = var;' pattern
                    if (parent.type === 'VariableDeclarator' && parent.id.type === 'ObjectPattern') {
                        parent.id.properties.forEach((property) => {
                            if (!property.key || property.key.name !== 'isSmallScreenWidth') {
                                return;
                            }
                            context.report({
                                node: property,
                                message,
                            });
                        });
                    }
                });

            },
            MemberExpression(node) {
                // Check for 'const isSmallScreen = useWindowDimensions().isSmallScreenWidth;' pattern
                if (node.object.type !== 'CallExpression' || node.object.callee.name !== 'useWindowDimensions' || node.property.name !== 'isSmallScreenWidth') {
                    return;
                }
                context.report({
                    node,
                    message,
                });
            },
        };
    },
};

we should also add a test for this rule.

What alternative solutions did you explore? (Optional)

N/A

[WebflowGuru]

Problem:
In the current application, there is a challenge when needing to use the same layout for both mobile devices and the right-hand panel (RHP) on wide screens. This problem arises because it's not always feasible to rely on the isSmallScreenWidth variable from useWindowDimensions to determine which layout to use. As a result, developers often resort to hard-coded and inconsistent solutions to address this issue.

Solution:
To address this problem, the following changes are proposed:

Route Prop for RHP Detection: In the navigation stack, it is suggested to add an isInRHP route prop to all screens within the RHP. This addition allows for easy identification of screens within the RHP.

Custom Hook - useResponsiveLayout: Create a custom hook called useResponsiveLayout to determine the appropriate layout based on the screen width and the isInRHP route prop.

function useResponsiveLayout() {
    const { isSmallScreenWidth } = useWindowDimensions();
    const { params } = useRoute();
    return {
        shouldUseNarrowLayout: isSmallScreenWidth || params.isInRHP,
    };
}

Replace useWindowDimensions with useResponsiveLayout: To ensure a consistent approach to layout handling, replace most of the current uses of useWindowDimensions with useResponsiveLayout as shown below:

// Before
const { isSmallScreenWidth } = useWindowDimensions();

// After
const { shouldUseNarrowLayout } = useResponsiveLayout();
Introduce a Lint Rule: To maintain code quality and consistency, introduce a new lint rule that discourages the use of isSmallScreenWidth from useWindowDimensions. Developers are advised to use shouldUseNarrowLayout instead.
This proposed solution aims to provide a more unified and maintainable approach to handling responsive layouts in the application. These changes simplify the management of layouts across various screen sizes and ensure a more consistent user experience.

[melvin-bot]

📣 @WebflowGuru! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[roryabraham]

Thanks for all the proposals everyone. At this time I've decided to move forward with the one that feels the clearest and most high-quality, which in my opinion is from @rayane-djouah here.

[rayane-djouah]

Thank you, I will raise a PR asap

[rayane-djouah]

PR ready for review: #50935

[alexpensify]

Waiting for this one to move to production

[melvin-bot]

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

[melvin-bot]

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

• [No QA] Bump eslint-config-expensify version #50935

If no regressions arise, payment will be issued on 2024-10-30. 🎊

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

• @getusha requires payment through NewDot Manual Requests
• @rayane-djouah requires payment (Needs manual offer from BZ)

[melvin-bot]

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:

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

[mountiny]

No regression tests required this is a tool for developers

The payment summary is $1000 to @rayane-djouah and to @getusha

[melvin-bot]

Payment Summary

Upwork Job

• Reviewer: @getusha owed $250 via NewDot
• ROLE: @rayane-djouah paid $(AMOUNT) via Upwork (LINK)

BugZero Checklist (@alexpensify)

• I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
• I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants/1717949899365408768/hired)
• I have paid out the Upwork contracts or cancelled the ones that are incorrect
• I have verified the payment summary above is correct

[alexpensify]

Payouts due: 2024-10-30

• Contributor: $1000 @rayane-djouah
• Reviewer: $1000 @getusha

Upwork job is here. I had to create a new job in Upwork. Please accept and I can complete the payment process. Thanks!

[rayane-djouah]

@alexpensify The payment amount for this issue is $1000 - #30528 (comment)

[rayane-djouah]

@alexpensify Offer accepted, thanks!

[alexpensify]

🤦🏼 good catch - Upwork was right. I've updated the summary here and paid @rayane-djouah via Upwork. I'm waiting for @getusha to accept.

[alexpensify]

Status: Waiting for @getusha to accept in Upwork

[rayane-djouah]

@alexpensify I think that @getusha is being paid in NewDot, we can close this issue

[alexpensify]

Thanks for flagging! This GH is from 2023. I think it needs to be paid via Chat, but let me double-check.

[alexpensify]

I confirmed that this one is from 2023. It will need to be paid via Upwork. I will move this to weekly since I'm OOO until Tuesday. @getusha, keep me posted when you accept it in Upwork. I'll complete the process when I'm back online.

[getusha]

@alexpensify accepted, thanks!

[alexpensify]

All set, everyone has been paid via Upwork: #30528 (comment)