[VISUAL ISSUE] Global create menu buttons don’t fill container on iPad

[jasperhuangg]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Upload.from.GitHub.for.iOS.MOV

Action Performed:

1. Open App on iPad.
2. Open the global create menu (green circle +).
3. Hold your finger on any button, notice how the hover styles don't apply to the entire length of the container.

Expected Result:

The button should flex grow to fill the entire container.

Actual Result:

The buttons have the same width they'd have on an iPhone.

Workaround:

N/A

Platform:

Where is this issue occurring?

• iOS

Version Number:
Reproducible in staging?:
Reproducible in production?:
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
Expensify/Expensify Issue URL:
Issue reported by:
Slack conversation:

View all open jobs on GitHub

[MelvinBot]

Triggered auto assignment to @tjferriss (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

[MelvinBot]

Current assignee @tjferriss is eligible for the AutoAssignerTriage assigner, not assigning anyone new.

[MelvinBot]

Triggered auto assignment to @puneetlath (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[jasperhuangg]

Adding external label as I am an engineer and can confirm that it can be fixed this way.

cc @puneetlath

[jasperhuangg]

Assigning Weekly as this is a visual issue, and is also really easy to fix.

[PrashantMangukiya]

Proposed Solution:

Here is the problem:
Within BasePopoverMenu.js at line 40 it set styles.createMenuContainer for large screen as under:

App/src/components/PopoverMenu/BasePopoverMenu.js

Line 40 in 36558e0

<View style={this.props.isSmallScreenWidth ? {} : styles.createMenuContainer}>

So createMenuContainer always set width: variables.sideBarWidth - 40, i.e. it will not show full width in large screen.

App/src/styles/styles.js

Lines 826 to 829 in 36558e0

	createMenuContainer: {
	width: variables.sideBarWidth - 40,
	paddingVertical: 12,
	},

Solution:
From index.native.js we need to pass containerStyles props (Not exist, we need to create) to BasePopoverMenu.js component, so it will make it full width as shown implementation below:

Add containerStyles props to src/components/PopoverMenu/BasePopoverMenu.js

const propTypes = {
     ...,
    // eslint-disable-next-line react/forbid-prop-types
    containerStyles: PropTypes.object,               // *** Add this code
    ...,
};

const defaultProps = {
    ...,
    containerStyles: {},                  // *** Add this code
};

// *** Update style as shown below 
<View style={this.props.isSmallScreenWidth ? {} : [styles.createMenuContainer, this.props.containerStyles]}>  
  ....
</View>

Add below code to src/components/PopoverMenu/index.native.js

return (
  <BasePopoverMenu
      // eslint-disable-next-line react/jsx-props-no-spreading
      {...this.props}
      onMenuHide={this.onMenuHide}
      onItemSelected={item => this.selectItem(item)}
      containerStyles={styles.w100}                    // *** Add this code
  />
);

Update below code to src/components/AttachmentPicker/index.native.js:

// *** Update style as shown below 
<View style={this.props.isSmallScreenWidth ? {} : [styles.createMenuContainer, styles.w100]}>
  ....
</View>

So this will solve problem everywhere for native iPad etc. For native small screen it behave as it is.

Below is screen record: It works as expected after this updates.

6453-iPad.mp4

[Santhosh-Sellavel]

@puneetlath @jasperhuangg

Fix will be no longer needed for this issue, global create menu width should not be of the full-screen width issue raised here #5754

The issue is fixed in the PR #6278 it's merged!

This Issue no longer exists now. Video for reference

iPad.mov

So let's close this.

[puneetlath]

Makes sense thanks @Santhosh-Sellavel.

And thanks for taking the time to make a proposal @PrashantMangukiya!

[Santhosh-Sellavel]

I see the PR #6278 Changes is reverted in PR #6523, because it caused a few deploy blockers, so we may need to reconsider this!

cc: @puneetlath @jasperhuangg

[parasharrajat]

#6278 would be reworked. It was reverted for regression. The author will fix the regression and resubmit.

[Santhosh-Sellavel]

#6278 would be reworked. It was reverted for regression. The author will fix the regression and resubmit.

Then we are good here!