mWeb - Navigation-Height of the navigation items in the Search tab did not update to 52px

[lanitochka17]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 9.0.54
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause - Internal Team

Issue found when executing PR #51169

Action Performed:

1. Go to Seach tab
2. Inspect on the web/desktop
3. Hover over the LHN nav items(Expenses, Chats, Invoices....)
4. Verify that height in 52px

Expected Result:

The height of the navigation items in the Search tab has been updated to 52px

Actual Result:

The height of the navigation items in the Search tab is 64 px instead of 52 px

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6647006_1729982278873.51169-mWeb.mp4

View all open jobs on GitHub

[melvin-bot]

Triggered auto assignment to @trjExpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[lanitochka17]

@trjExpensify FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[huult]

Edited by proposal-police: This proposal was edited at 2024-10-28 06:37:31 UTC.

Proposal

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

Height of the navigation items in the Search tab did not update to 52px

What is the root cause of that problem?

We used the PopoverMenu component to render a navigation item in the search. Inside it, FocusableMenuItem is used to render each item, and within FocusableMenuItem, MenuItem is utilized. The combinedStyle has a paddingVertical value of 12, and the wrap title content uses a minHeight of 40, making the total height of each item 64.

App/src/components/MenuItem.tsx

Line 591 in 5103b4b

combinedStyle,

and

App/src/components/MenuItem.tsx

Line 719 in 5103b4b

StyleUtils.getMenuItemTextContainerStyle(isSmallAvatarSubscriptMenu),

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

To resolve this issue, we just need to update the wrapperStyle for popoverMenuItems by setting paddingVertical in the MenuItem, like this

//.src/pages/Search/SearchTypeMenuNarrow.tsx#L79
  const popoverMenuItems = useMemo(() => {
        const items = typeMenuItems.map((item, index) => {
            const isSelected = title ? false : index === activeItemIndex;

            return {
                ...
+              wrapperStyle: [styles.pv1half],
            };
        });

        if (title && !currentSavedSearch) {
            items.push({
                ...
+              wrapperStyle: [styles.pv1half],
            });
        }

        return items;
-    }, [typeMenuItems, title, activeItemIndex, singleExecution, theme, policyID, closeMenu, currentSavedSearch]);
+   }, [typeMenuItems, title, currentSavedSearch, activeItemIndex, singleExecution, theme.iconSuccessFill, theme.icon, theme.border, styles.pv1half, policyID, closeMenu]);

What alternative solutions did you explore? (Optional)

To resolve this issue and avoid impacting other uses of PopoverMenu, we should create a new parameter to update paddingVertical to 6. An example of the code change is as follows:

//.src/pages/Search/SearchTypeMenuNarrow.tsx#214
            <PopoverMenu
                ...
+                isSearchTypeMenuNarrow
            />
//.src/components/PopoverMenu.tsx#L159
    function PopoverMenu({
      ...
+     isSearchTypeMenuNarrow = false,
    }: PopoverMenuProps) {

//.src/components/PopoverMenu.tsx#L309
    <FocusableMenuItem
        ...                           
+        isSearchTypeMenuNarrow={isSearchTypeMenuNarrow}
        ...
    />
//.src/components/MenuItem.tsx#L444
function MenuItem(
    {
        ...
+        isSearchTypeMenuNarrow = false,
    }: MenuItemProps,
    ref: PressableRef,
) 
//.src/components/MenuItem.tsx#L592
    style={({pressed}) =>
        [
            containerStyle,
            combinedStyle,
+            !!isSearchTypeMenuNarrow && [styles.pv1half],
            !interactive && styles.cursorDefault,
            ...
        ] as StyleProp<ViewStyle>
    }

POC

-

[trjExpensify]

CC: @jayeshmangwani @s77rt @shawnborton @rafecolton from the linked PR, is this a regression to fix, missed or intentional?

[jayeshmangwani]

@trjExpensify This is intentional; we decided not to change the height for the Search popover (on mobile devices Android, ios, mobile web), as it will be addressed when we update the styling for all popovers across the app.

[shawnborton]

That's correct, we can just close this out. cc @Expensify/design

[dannymcclain]

Agree—this is expected.

[trjExpensify]

Great stuff, banished!