-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
[Due for payment 2025-03-11] [$250] Status - With status message entered, save button hidden behind keypad on opening #53692
Comments
Triggered auto assignment to @lschurr ( |
Job added to Upwork: https://www.upwork.com/jobs/~021866149099136907717 |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @allgandalf ( |
Edited by proposal-police: This proposal was edited at 2023-10-07T06:34:00Z. ProposalPlease re-state the problem that we are trying to solve in this issue.Status - With status message entered, save button hidden behind keypad on opening. What is the root cause of that problem?the issue happens when the clear status element is added after we type something in, that element takes space and so the save button will be pushed down for lack of space. What changes do you think we should make in order to solve the problem?check if clear status element should be present:
and only if
and obviously, we use the shouldShowClearStatus instead in the component: App/src/pages/settings/Profile/CustomStatus/StatusPage.tsx Lines 213 to 222 in 64d6c38
What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?UI issue What alternative solutions did you explore? (Optional) |
Edited by proposal-police: This proposal was edited at 2024-12-16 15:04:30 UTC. ProposalPlease re-state the problem that we are trying to solve in this issue.With status message entered, save button hidden behind keypad on opening What is the root cause of that problem?The RCA of this issue is that the available height is not sufficient to display all elements when the keyboard opens. As a result, the keyboard overlaps the button, and scrolling is required to access it. Screen.Recording.2024-12-11.at.21.49.24.movWhat changes do you think we should make in order to solve the problem?To resolve this issue, we should scroll the form to the end when the keyboard opens to prevent the keyboard from overlapping the button, ensuring the button is visible as expected. To implement this, follow the steps below:
App/src/components/Form/FormWrapper.tsx Line 41 in 25127ff
function FormWrapper({
onSubmit,
children,
errors,
inputRefs,
submitButtonText,
footerContent,
isSubmitButtonVisible = true,
style,
submitButtonStyles,
submitFlexEnabled = true,
enabledWhenOffline,
isSubmitActionDangerous = false,
formID,
shouldUseScrollView = true,
scrollContextEnabled = false,
shouldHideFixErrorsAlert = false,
disablePressOnEnter = false,
isSubmitDisabled = false,
shouldScrollToEnd = false,
}: FormWrapperProps) { useEffect(() => {
if (!shouldScrollToEnd) {
return;
}
InteractionManager.runAfterInteractions(() => {
requestAnimationFrame(() => {
formRef.current?.scrollToEnd({animated: true});
});
});
}, [shouldScrollToEnd]);
const {keyboardHeight} = useKeyboardState(); <FormProvider
formID={ONYXKEYS.FORMS.SETTINGS_STATUS_SET_FORM}
style={[styles.flexGrow1, styles.flex1]}
ref={formRef}
submitButtonText={translate('statusPage.save')}
submitButtonStyles={[styles.mh5, styles.flexGrow1]}
onSubmit={updateStatus}
validate={validateForm}
enabledWhenOffline
shouldScrollToEnd
// shouldScrollToEnd={keyboardHeight !== 0}
>
POCScreen.Recording.2024-12-11.at.22.57.34.mp4What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?This is a UI bug; I don’t think we need to test it What alternative solutions did you explore? (Optional)When the text input is focused, we scroll it to the top to maximize the visibility of the elements below.
App/src/components/Form/FormProvider.tsx Lines 265 to 267 in bfe4e35
App/src/components/Form/FormWrapper.tsx Lines 106 to 119 in bfe4e35
App/src/pages/settings/Profile/CustomStatus/StatusPage.tsx Lines 210 to 214 in bfe4e35
POCScreen.Recording.2024-12-16.at.23.28.44.movTest branchReminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job. |
@allgandalf here is pervious similar issue #50073 |
@allgandalf Because the height of the page is not enough to contain all the elements, they are covered when the scroll appears. |
@huult sorry i don't think introducing a new prop is what we want for this issue, so we shouldn't move with that approach, is there any alternative solution you have thought about? @M00rish have you tested your solution ? It didn't work for me, can you share a result video if your suggestions fix this issue ? |
This comment was marked as outdated.
This comment was marked as outdated.
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸 |
I thought it's like the other issue but it's not actually, I have updated my proposal @allgandalf. |
Proposal updated
@allgandalf Could you check again? |
@huult my main concern with the issue is that we shouldn't apply patches here (meaning workarounds) because this is a page specific issue and not component specific. If there is no clean solution, we can surely consider your proposal, I'll ask on open source channel for more ideas, please consider other ideas too, thanks :) update: Posted for new proposals here |
ProposalPlease re-state the problem that we are trying to solve in this issue.With status message entered, save button hidden behind keypad on opening status page. What is the root cause of that problem?The form's scroll view contains all elements including the submit button. When the keyboard is opened if the content doesn't have enough height to display, the submit button is overlapped with the keyboard and we need to scroll to see the full submit button. App/src/components/Form/FormWrapper.tsx Lines 171 to 180 in d86024e
What changes do you think we should make in order to solve the problem?We should move the submit button to the outside to make sure it always displays at the bottom and behind the keyboard App/src/components/Form/FormWrapper.tsx Lines 171 to 180 in d86024e
App/src/components/Form/FormWrapper.tsx Line 101 in d86024e
App/src/components/Form/FormWrapper.tsx Line 161 in d86024e
Note: This is a big change in global form component, we need to test carefully in the PR phrase to make sure the UI is good for all forms. What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?NA (it's a UI issue) What alternative solutions did you explore? (Optional)Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job. ResultScreen.Recording.2024-12-17.at.15.07.25.mov |
@allgandalf What do you think about my proposal? |
I hope you can review my updated one too thanks. |
@allgandalf I hope you consider this option because my proposal reduces regression testing and involves only minimal changes. |
@lschurr @allgandalf 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! |
Had problem setting up small screen for android, hopeful to get it fixed today and continue with reviewing |
I like the approach from @huult here, RCA is correct and solution seems more feasible. @mkzie2 , I tried applying your solution but it is way too complex and affects the app entirely also i don't like the idea of moving the button out of scrollview, so yeah lets go with @huult proposal here 🎀👀🎀 C+ reviewed |
Triggered auto assignment to @pecanoro, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸 |
Assigning @huult to the issue |
📣 @allgandalf 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app! |
📣 @huult 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app! Offer link |
Thank you all! I will create the pull request within 24 hours. |
@allgandalf could you check the pull request? |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.1.8-1 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2025-03-11. 🎊 For reference, here are some details about the assignees on this issue:
|
@allgandalf @lschurr @allgandalf The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button] |
@allgandalf could you work on the checklist for this one? |
sure, will complete tomorrow, please put this back to daily |
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Version Number: V9. 0.72-0
Reproducible in staging?: Yes
Reproducible in production?: Yes
Issue reported by: Applause Internal Team
Action Performed:
Expected Result:
With status message entered, save button must not be hidden behind keypad on opening status page.
Actual Result:
With status message entered, save button hidden behind keypad on opening status page.
Workaround:
Unknown
Platforms:
Screenshots/Videos
Bug6686106_1733471673686.Screenrecorder-2024-12-06-13-15-46-740_compress_1.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @lschurrThe text was updated successfully, but these errors were encountered: