[$2000] mWeb/Safari -The list of countries opens in different sizes when clicking on different lines

[kbecciv]

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

---

Action Performed:

1. Log in with a brand new account in iOS/ Safari
2. Navigate to Settings -> Profile -> Personal details->Home address
3. Click Country (a large file will open)
4. Сlick Address line 1 then click Country again. (a small sheet opens)

Expected Result:

The sheet with the list of countries should open in the same size

Actual Result:

The list of countries opens in different sizes when clicking on different lines

Workaround:

Uknown

Platforms:

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

• [ ] Android / native
• [ ] Android / Chrome
• [ ] iOS / native
• [x] iOS / Safari
• [ ] MacOS / Chrome / Safari
• [ ] MacOS / Desktop

Version Number: 1.2.87.0

Reproducible in staging?: Yes

Reproducible in production?: Yes

If this was caught during regression testing, add the test name, ID and link from TestRail:

Email or phone of affected tester (no customers):

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

https://user-images.githubusercontent.com/93399543/226010101-7e3e3202-7ad3-4132-85c6-0e59b327df98.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause- Internal Team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~011bc5d3c6e8254d08
• Upwork Job ID: 1637850559436447744
• Last Price Increase: 2023-04-03

[MelvinBot]

Triggered auto assignment to @anmurali (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[MelvinBot]

Bug0 Triage Checklist (Main S/O)

• [x] This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• [x] This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• [x] This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• [x] This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• [x] I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[anmurali]

Reproduced only on iOS/Safari

[MelvinBot]

Job added to Upwork: https://www.upwork.com/jobs/~011bc5d3c6e8254d08

[MelvinBot]

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

[MelvinBot]

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

[MelvinBot]

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

[adelekennedy]

It's not the same issue but these two seem like they could have a similar fix?

[hohner2008]

For the sorting issue we must use .sortBy instead of .map .

[MelvinBot]

📣 @hohner2008! 📣

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. 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.
2. 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.
3. 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>

[s77rt]

@hohner2008 Thanks for your interest but can you please follow the proposal template? (checkout contributing guide)

[hellohublot]

Proposal

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

mWeb/Safari -The list of countries opens in different sizes when clicking on different lines

What is the root cause of that problem?

When switching from an input to a select, the keyboard is not hidden yet, so the select popup list box can only occupy half of the screen height

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

We can hide the keyboard before popping up the list box. In fact, the browser will hide the keyboard, but we hide the keyboard in advance so that the browser can correctly calculate the height of the list box

https://github.com/Expensify/react-native-picker-select/blob/84ee97dec11c2e65609511eb5a757d61bbeeab79/src/index.js#L580-L592

     selectedValue={selectedItem.value}
     {...pickerProps}
+    onPointerUp={(event) => {
+        Keyboard.dismiss();
+        if (pickerProps.onPointerUp) {
+            pickerProps.onPointerUp(event);
+        }
+    }}

What alternative solutions did you explore? (Optional)

None

[aswin-s]

Proposal

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

Country picker dropdown size differs when soft keybord is open on iOS Safari.

What is the root cause of that problem?

When user directly taps on the country picker, soft keyboard is not present and browser makes use of entire browser height to render the select dropdown. Once user focus any of the other inputs, soft keyboard opens up and reduces the available screen height of the page. Now if user tries to open country picker browser renders a smaller select dropdown because of reduced screen height.

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

This issue is not specific to Country picker but applicable to Picker component in general. Soft keyboard should be dismissed before opening select dropdown. Otherwise browser will render a smaller picker component. Picker component is internally using react-native-picker-select which programatically dismiss the Keyboard before opening the modal for platforms other than Web. For web, a native 'Select' input is getting rendered. So we need to close the soft input before opening picker. This could be achieved by listening to onTouchEnd event from picker and blurring active element if it is not select input.

Add touchEnd listener to pickerProps like below

pickerProps={{
onFocus: this.enableHighlight,
...
onTouchEnd: (e) => {
  if (document.activeElement !== e.target) {
   document.activeElement.blur();
 }
}
},

After Fix

https://user-images.githubusercontent.com/6880914/226537255-37ce8144-48f8-4e2e-8b32-ce5cf8c82558.mp4

[MelvinBot]

📣 @aswin-s! 📣

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. 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.
2. 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.
3. 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>

[aswin-s]

Contributor details Your Expensify account email: mr.aswin@gmail.com Upwork Profile Link: https://www.upwork.com/freelancers/~010b194c3e145d5456

[MelvinBot]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[hohner2008]

Contributor details Your Expensify account email: hohner2008@gmail.com Upwork Profile Link: https://www.upwork.com/freelancers/~0150d8e3bbd1d46f2b

[MelvinBot]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[s77rt]

@hellohublot Thanks for the proposal. Your RCA seems about right. The idea of hiding the keyboard also makes sense to me. What I'm concerned about is when and where to hide the keyboard. Let's get this to the lowest level possible, and dismiss the keyboard just before opening the select menu.

[s77rt]

@aswin-s Thanks for the proposal. Your RCA is correct and the fix of hiding the keyboard before opening the select menu is probably what we are looking for here. I'm interested in knowing more about this:

Picker component is internally using react-native-picker-select which programatically dismiss the Keyboard before opening the modal for platforms other than Web.

Can you link to the code? Why this is not being handled for Web. It would be best to extend the functionality and make it work for Web too (upstream fix).

[aswin-s]

@s77rt

react-native-picker-select dismiss the native keyboard whenevertogglePicker method is invoked for iOS.

https://github.com/lawnstarter/react-native-picker-select/blob/ca6488c2eef5c776a8071400c8b7987712d43397/src/index.js#L239

For Android it doesn't seem to dismiss keyboard, as the picker opens above the SoftKeyboard. However there is an open PR to fix this behaviour.

For Web platform, it is listening only to the onValueChange event from Select input. This is because RNPIckerSelect internally uses react-native-picker for rendering the Select component. ReactNativePicker has separate implementations for iOS, Android & Web. For Web it simply returns a browser native Select input.

Render method for Web platform in react-native-picker select https://github.com/lawnstarter/react-native-picker-select/blob/master/src/index.js#L563-L565

Web implementation for react-native-picker https://github.com/lawnstarter/react-native-picker-select/blob/master/src/index.js#L563-L565

Select input on browser doesn't need any additional plumbing as selection happens at browser level and onChange listener will be triggered once user picks a value from select input. onChange listener in turn calls onValueChange callback for react-native-picker component.

https://github.com/react-native-picker/picker/blob/master/js/Picker.web.js#L59

So ideally the bug should be fixed in 'react-native-picker' repo as it is the component responsible for rendering select input. I can raise a PR at the upstream repo and link it back here.

[s77rt]

@aswin-s Thanks for the quick update. We have a fork of react-native-picker-select. Do you think we can fix this there? Or to make my question more straightforward, what change should we make in either react-native-picker or react-native-picker-select to fix the bug?

[aswin-s]

@s77rt If we already have a fork of react-native-picker-select that would suffice. All we need to do is listen to onTouchEnd or onMouseDown event from the select input and blur the activeElement if the activeElement is not the select input itself. Both these events getFired before the 'click' event on select input and gives us an opportunity to blur any other inputs and dismiss the soft keyboard.

In short we should modify the renderWeb method in react-native-picker-select as shown below to inject onTouchEnd listener.

renderWeb() {
        const { disabled, style, pickerProps } = this.props;
        const { selectedItem } = this.state;

        // Extract onTouchEnd callback from pickerProps
        const {onTouchEnd,...pickerPropsExceptOnTouchEnd} = pickerProps;

       // Add handler for onTouchEnd event
        const handleTouchEnd = (e)=>{
            // ignore if event is triggered from select input itself
            if (document.activeElement === e.target) { return; }
            // otherwise blur the element so that soft keyboard gets dismissed
            document.activeElement.blur();
            // call onTouchEnd callbacks from child component if any
            pickerProps?.onTouchEnd?.(e);
        };

        return (
            <View style={[defaultStyles.viewContainer, style.viewContainer]}>
                <Picker
                    style={[style.inputWeb]}
                    testID="web_picker"
                    enabled={!disabled}
                    onValueChange={this.onValueChange}
                    selectedValue={selectedItem.value}
                    onTouchEnd={handleTouchEnd}
                    {...pickerPropsExceptOnTouchEnd}
                >
                    {this.renderPickerItems()}
                </Picker>
                {this.renderIcon()}
            </View>
        );
    }

[s77rt]

@aswin-s Thanks for the update. Although onTouchStart/onTouchEnd may work they are not strictly coupled with opening the select menu. Can we get closer to the event that occurs right before opening the select menu?

[hellohublot]

Proposal

Updated

@s77rt

Hello, I tested again, onTouchStart will conflict with the scroll event, this event will also respond outside the area, so I replaced it with onPointerUp

When to hide the keyboard

I tested ['onFocus', 'onFocusIn', 'onMouseUp', ''onTouchStart', "onPointerUp"', "onFullscreenChange"], in these events, focus and mouse are too late, touch will affect the scroll event, so I would recommend onPointerUp, onPointerUp will only respond in the browser, so it will not affect other scopes

Where to hide the keyboard

In fact, there are onFocus onBlur and additionalPickerEvents here, I suggest that we deal with it in this code block is the best, this is a problem that exists with input and select, I think the main thing in the upstream code react-native-picker-select is only do select, And we will facilitate the processing of other possible modals except input in the future

How to hide the keyboard

I suggest that we call the official method of react-native, which will handle a lot of details for us Keyboard.dismiss() -> dismissKeyboard() -> TextInputState.blurTextInput(TextInputState.currentlyFocusedField()) -> if (textFieldNode !== null && document.activeElement === currentlyFocusedField) -> blur() so Keyboard.dismiss() does not have other effects

[s77rt]

@hellohublot I didn't really understand the part of where to apply the fix. Please provide a permlink. Also can you please update your proposal here with your new suggested changes instead of posting new comments.

[s77rt]

@hellohublot I believe we want the fix to be upstream on our fork https://github.com/Expensify/react-native-picker-select. Any reasons you think it's better to apply the fix only on E/App repo?

[hellohublot]

@s77rt

@hellohublot Thanks for the proposal. Your RCA seems about right. The idea of hiding the keyboard also makes sense to me. What I'm concerned about is when and where to hide the keyboard. Let's get this to the lowest level possible, and dismiss the keyboard just before opening the select menu.

Hello, I have modified the original proposal, I also linked to the original proposal, I just explain where I modified (because you have read the original proposal and asked some questions), and answer you For some problems with the original proposal, my permlink remains unchanged in the original proposal. I am very sorry for the trouble. If you agree, I will send two messages when I update the proposal next time

---

@hellohublot I believe we want the fix to be upstream on our fork https://github.com/Expensify/react-native-picker-select. Any reasons you think it's better to apply the fix only on E/App repo?

Not yet

[s77rt]

@hellohublot Can you update your proposal based on the fact that we want to apply the fix upstream.

[aswin-s]

@aswin-s Thanks for the update. Although onTouchStart/onTouchEnd may work they are not strictly coupled with opening the select menu. Can we get closer to the event that occurs right before opening the select menu?

@s77rt Since HTML Select element is getting rendered inside Picker component for web platform, only limited set of events are fired when user interacts with the picker. Below are the list of events that are getting fired in the exact same order.

pointerover
pointerenter
pointerdown
touchstart
gotpointercapture
pointerup
lostpointercapture
pointerout
pointerleave
touchend
mouseover
mouseenter
mousemove
mousedown
focus
mouseup
click
animationstart
animationend

The picker menu gets opened once click event is triggered. So we need to blur any other active input and dismiss the keyboard before focus and click gets triggered on select element. As the issue is happening only on mobile devices it looks like touchend is the closest event that gets triggered before opening the select menu.

We could wrap the select element in a TouchableOpacity and capture any touch events that happens inside it. But to open the Select menu after the cleanup we'll have to manually trigger a click event on the Select element. This feels like too much of a hack for such a small issue.

[s77rt]

@aswin-s Thanks for the update. If the picker is opened on click event, can't we dismiss the keyboard on that event?

[aswin-s]

@aswin-s Thanks for the update. If the picker is opened on click event, can't we dismiss the keyboard on that event?

Nope, as you can see from the order of events above, focus event on select element gets fired before click event. So if we try to blur the active element on click, that would be select itself. We need to blur the input even before focus event on select so that soft keyboard gets dismissed and browser takes entire screen height for computing select menu height.

[s77rt]

@aswin-s Thanks for the clarification, but won't touchEnd fire if we are scrolling the view and happen to land on the picker input?

[aswin-s]

@aswin-s Thanks for the clarification, but won't touchEnd fire if we are scrolling the view and happen to land on the picker input?

@s77rt If user tries to scroll the page by pressing and scrolling over select input touchEnd will be fired. For this to happen scroll has to start from select input itself. If that happens touchMove will also be triggered before touchEnd. We can listen totouchMove event and cancel actions on touchEnd.

[s77rt]

@aswin-s Thanks for the follow up. I'm really trying to be fair here but I find it hard to go with your proposal, I don't see a strong argument on using onTouchEnd over onPointerUp. Also I find it much cleaner to use Keyboard.dismiss() instead of the blur approach you suggested.

[s77rt]

@aswin-s Thanks again for understanding. I appreciate your inputs here. I'm sure you can find more issues that you can work with, there is plenty. Checkout issues with Help Wanted label.

[s77rt]

@hellohublot Given that you were the first person to get the correct RCA and proposed the appropriate fix. I think we will go with your proposal. However we are expecting the fix to be upstream on our fork here https://github.com/Expensify/react-native-picker-select.

:ribbon: :eyes: :ribbon: C+ reviewed

cc @AndrewGable

[s77rt]

@AndrewGable I want to add that the approved proposal ^ has one slight drawback:

• Current behavior: On long press the picker, nothing happens
• New behavior: On long press the picker, the keyboard will be dismissed.

Difference in videos

| Current behavior | New behavior | |-------------------|---------------| |

I think this is an edge case and not a blocker just worth mentioning to be clear. This is happening because pointerup event is not strictly coupled with opening the select menu, focus and click are but acting on those events is too late. pointerup is what looks best so far.

[hellohublot]

Proposal

Updated

@s77rt I took a look at the upstream code, thanks for your suggestion, I have put the changes into the upstream code, and updated my proposal

[s77rt]

Not overdue. Waiting for @AndrewGable ^ @tjferriss Can you please revert the price $1K

[AndrewGable]

Sorry @s77rt - I was looking into this comment and got distracted, doubling is fine. Can you please elaborate on this edge case? Does the updated proposal fix this?

[s77rt]

@AndrewGable I'm not sure how to elaborate on this one. Did you check the attached videos explaining the difference?

Does the updated proposal fix this?

No, this is related to the event that we act on, it's still pointerup

[AndrewGable]

I got distracted because I'm not sure we are "OK" with this edge case, I cannot think of a current case in which we want this specific behavior, but I am not 100% sure we want to dismiss the keyboard in all cases. I will investigate a bit further but we probably want to discuss this in the open source Slack room.

[MelvinBot]

Upwork job price has been updated to $1000

[s77rt]

@AndrewGable Please leave a link to Slack if a discussion has been opened.

[tjferriss]

@anmurali it looks like I was also assigned to this one somehow after you were initially assigned as the BZ team member. I'm going to un-assign myself now. Let me know if I'm misunderstanding.

[MelvinBot]

@AndrewGable @anmurali @s77rt this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[s77rt]

We have a proposal already but with a slight drawback. Waiting for @AndrewGable's feedback

[s77rt]

Same ^

[AndrewGable]

Ok reviewed the proposal and it seems fine. @hellohublot - Please submit the PR to both the fork and upstream repo.