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

FAB hidden by Keyboard on iPad #2151

Closed
JmillsExpensify opened this issue Mar 30, 2021 · 17 comments · Fixed by #2183
Closed

FAB hidden by Keyboard on iPad #2151

JmillsExpensify opened this issue Mar 30, 2021 · 17 comments · Fixed by #2183
Assignees

Comments

@JmillsExpensify
Copy link

JmillsExpensify commented Mar 30, 2021

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


Expected Result:

The FAB should always be visible so users can easily trigger their desired action, on every platform

Actual Result:

FAB hidden by keyboard on iPad

Action Performed:

  1. Log in to E.cash with iPad
  2. Reveal software keyboard
  3. Observe FAB menu is not visible

Workaround:

None

Platform:

iOS (iPad)
Screen Shot 2021-03-29 at 12 56 06 PM

Version Number:
v1.0.7-0
Logs: None
Notes/Photos/Videos: See above
Expensify/Expensify Issue URL: https://github.com/Expensify/Expensify/issues/158758
Upwork link: https://www.upwork.com/jobs/~0158125c819a6e8fac

@JmillsExpensify
Copy link
Author

@marcaaron this is my first triage, let me know if I missed something!

@marcaaron
Copy link
Contributor

Looks perfect !

@aliabbasmalik8
Copy link
Contributor

aliabbasmalik8 commented Mar 30, 2021

The solution is just needed to add KeyboardAvoidingView in FAB component like this

            <KeyboardAvoidingView behavior="position">
                <AnimatedPressable
                    onPress={this.props.onPress}
                    style={[
                        styles.floatingActionButton,
                        {transform: [{rotate}], backgroundColor},
                    ]}
                >
                    <AnimatedIcon src={Plus} fill={fill} />
                </AnimatedPressable>
            </KeyboardAvoidingView>

image

@marcaaron
Copy link
Contributor

Alright sounds good. Please let's make sure to also show screenshots of this working ok on all platforms when the PR is ready (tablet Android too thanks).

@JmillsExpensify
Copy link
Author

Hi @aliabbasmalik8, I wanted to confirm that you added a proposal on Upwork? You'll need to do that so we can hire you officially for the job. Thank you!

@aliabbasmalik8
Copy link
Contributor

@JmillsExpensify Proposal submitted on Upwork.
Thanks

@JmillsExpensify
Copy link
Author

Awesome, thank you!

@JmillsExpensify
Copy link
Author

Thanks @aliabbasmalik8. We settle Upwork jobs 7 days after the PR is merged to ensure no regressions.

@marcaaron
Copy link
Contributor

There are a couple of issues with this PR so I'm reopening until they are resolved.

@JmillsExpensify
Copy link
Author

Changes requested on the linked PR.

@JmillsExpensify
Copy link
Author

PR merged on Apr. 13. Waiting for any regressions until April 19, then we'll issue payment.

@JmillsExpensify
Copy link
Author

Issuing payment via Upwork, as we haven't seen any regressions.

@melvin-bot
Copy link

melvin-bot bot commented Oct 4, 2022

⚠️ Looks like this issue was linked to a possible regression on PRODUCTION here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a production regression has occurred a Root Cause Analysis is required. Please follow the instructions here.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

@JmillsExpensify
Copy link
Author

Hmm, I'll have to look into a possible regression on this one. That said, I'm actually not sure if this is truly a regression, as these comments can get kind of spammy.

@marcaaron
Copy link
Contributor

Agree it's not a regression - the comment happens anytime we mention any issue so we can at least attempt to see if they are mentioning something about it being a regression. Looks like the contributor is just linking to this PR to explain when the original code was added - so not likely that this super old issue has caused any recent regression 😄

I think we are going to remove this, because everyone hates it. I don't really understand why it gets so much hate and think we have to do something to try to figure out if things are regressions otherwise we will never do RCA for anything.

@melvin-bot
Copy link

melvin-bot bot commented Oct 5, 2022

⚠️ Looks like this issue was linked to a possible regression on PRODUCTION here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a production regression has occurred a Root Cause Analysis is required. Please follow the instructions here.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

@JmillsExpensify
Copy link
Author

I think we are going to remove this, because everyone hates it. I don't really understand why it gets so much hate and think we have to do something to try to figure out if things are regressions otherwise we will never do RCA for anything.

Oh to be clear, I don't hate them. I agree, it's net good and worth the time. Probably what would make this more helpful (and less confusing) is an expectation around issue linking. A lot of people link for context rather than to point to regressions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants