-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
[HOLD for payment 2024-10-14] [$250] The intro text on private notes list stays fixed when it should scroll with the page #49298
Comments
Triggered auto assignment to @lschurr ( |
ProposalPlease re-state the problem that we are trying to solve in this issue.Gives error saying "Please enter a valid website using lowercase characters" What is the root cause of that problem?We are not adding this App/src/pages/PrivateNotes/PrivateNotesListPage.tsx Lines 98 to 104 in 1156e97
What changes do you think we should make in order to solve the problem?We can create a function like
What alternative solutions did you explore? (Optional)NA |
Job added to Upwork: https://www.upwork.com/jobs/~021835811668345209683 |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @brunovjk ( |
Hey @brunovjk I would like to work on this issue. |
📣 @vr-varad! 📣
|
Contributor details |
✅ Contributor details stored successfully. Thank you for contributing to Expensify! |
📣 @markom01! 📣
|
Contributor details |
|
Contributor details |
✅ Contributor details stored successfully. Thank you for contributing to Expensify! |
ProposalPlease re-state the problem that we are trying to solve in this issue.The intro text should be attached to the content of the page and scroll with private notes text section. What is the root cause of that problem?The personalNoteMessage text is outside of the ScrollView as What changes do you think we should make in order to solve the problem?We can move the personalNoteMessage text inside the ScrollView as below.
What alternative solutions did you explore? (Optional)The current design is aligned with pages like Profile description page and Room description page. So, I am not sure if we should alter the behavior for this page. Profile description page also limits text to 1000 characters if we should consider this for this case. |
Performed a minor edit to the problem statement. |
Edited by proposal-police: This proposal was edited at 2024-09-18 14:00:37 UTC. ProposalPlease re-state the problem that we are trying to solve in this issue.In PrivateNotesListPage.tsx What is the root cause of that problem?
What changes do you think we should make in order to solve the problem?Add <ScrollView contentContainerStyle={styles.flexGrow1} bounces={false}>
<Text style={[styles.mb5, styles.ph5]}>{translate('privateNotes.personalNoteMessage')}</Text>
{privateNotes.map((item) => getMenuItem(item))}
</ScrollView> What alternative solutions did you explore? (Optional)Make no changes on web (introduction text stays fixed) and fix introduction text to top while scrolling on other platforms for consistency. Recording of proposalfix.mov |
ProposalIn the current code implementation:
The Proposed SolutionTo fix this issue, the text should be moved inside the . This way, the text will scroll along with the list of private notes. The updated code should look like this:
Explanation of Changes:
|
I think this is an expected behavior as the intro text is an informative text about the notes that is not part of the private notes list. |
I agree with @FitseTLT, but then consistency can be problem, because for example, on iOS that text is in scrollview. ios-scroll.mp4 |
Thank you all for the proposals and investigation. I am having problems with my internet, the company is coming to replace my modem today. If this issue is urgent, please reassign it. Thank you very much. |
@nkdengineer's proposal to move the introductory text inside the Before proceeding with the fix, we would like to confirm:
Once we have confirmation, we can move forward with either applying the fix or leaving the current behavior unchanged. Thank you! 🎀👀🎀 C+ reviewed |
Thanks @MonilBhavsar and @brunovjk. I further looked into the similar scenarios like Room description and Workspace profile description.
Please refer the screen recording explaining this for a large screen and ios screen. I could not attach this due to larger file size. https://drive.google.com/file/d/1_vTxA5MtDCn8w2Nc6395AHfieY4mQ4BU/view?usp=sharing So, I feel making the below update(same as proposed earlier) will handle the case. This will be aligned with the discussed behavior as well as consistant with the other pages.
|
@MonilBhavsar I double checked other pages like Room description, Workspace profile description, ... and the intro text scrolls with the comment, the intro text doesn't scroll in only Private notes |
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸 |
@MonilBhavsar @lschurr @brunovjk 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! |
Not overdue. We asked on Slack for opinions on how to proceed here. We are still discussing it :D |
Thanks all! Assigning @nkdengineer as they were the first to put up a proposal and proposal remains unchanged |
📣 @brunovjk 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app! |
📣 @nkdengineer 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app! Offer link |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.45-4 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 2024-10-14. 🎊 For reference, here are some details about the assignees on this issue:
|
BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
|
BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
Regression Test Proposal
Do we agree 👍 or 👎? cc: @lschurr |
Payment summary:
|
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.36-1
Reproducible in staging?: Y
Reproducible in production?: Y
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
Expensify/Expensify Issue URL:
Issue reported by: @dukenv0307
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1726513353005399
Action Performed:
Expected Result:
The intro text should be attached to the content of the page and scroll like we did with many pages
Actual Result:
The intro text is fixed when you scroll the page
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
bug.1.mov
Recording.553.mp4
Add any screenshot/video evidence
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: