-
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
[$250] The intro text on More Features stays fixed when it should scroll with the page #49075
Comments
Triggered auto assignment to @abekkala ( |
Edited by proposal-police: This proposal was edited at 2024-09-13 08:13:38 UTC. ProposalPlease re-state the problem that we are trying to solve in this issue.The intro text on More Features stays fixed when it should scroll with the page What is the root cause of that problem?
What changes do you think we should make in order to solve the problem?
<Text style={[styles.ph5, styles.mb4, styles.mt3, styles.textSupporting, shouldUseNarrowLayout ? styles.workspaceSectionMobile : styles.workspaceSection]}>
{translate('workspace.moreFeatures.subtitle')}
</Text>
{!shouldUseNarrowLayout && getHeaderText()}
<ScrollView contentContainerStyle={styles.pb2}>
{shouldUseNarrowLayout && getHeaderText()}
{sections.map(renderSection)}
</ScrollView> App/src/pages/workspace/WorkspaceMoreFeaturesPage.tsx Lines 412 to 416 in caf529b
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. |
ProposalPlease re-state the problem that we are trying to solve in this issue.The intro text on More Features stays fixed when it should scroll with the page What is the root cause of that problem?We are not adding this Text in App/src/pages/workspace/WorkspaceMoreFeaturesPage.tsx Lines 412 to 414 in 04762ac
What changes do you think we should make in order to solve the problem?We can create a const
and then outside scrollView here we can add
and then finally we can change scrollView
Note: We might need some minor styling that can be done in PR. What alternative solutions did you explore? (Optional)We can use |
ProposalUpdated added additional changes we should make for more consistent user experience. Reference images for proposal
|
@Expensify/design
Should |
The More features part should stay fixed. |
That's what I was assuming |
Agree with Jon, I think this is something we need to fix. |
OH! I did misunderstand. I thought you meant the whole "More Features" part stays fixed! |
Job added to Upwork: https://www.upwork.com/jobs/~021837165152252710251 |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @suneox ( |
Edited by proposal-police: This proposal was edited at 2024-09-21 01:17:03 UTC. ProposalPlease re-state the problem that we are trying to solve in this issue.
What is the root cause of that problem?
App/src/pages/workspace/WorkspaceMoreFeaturesPage.tsx Lines 412 to 414 in 04762ac
What changes do you think we should make in order to solve the problem?
What alternative solutions did you explore? (Optional) |
@dubielzyk-expensify Can you help confirm that: This issue need to be resolved on all platforms, right? |
I think it should be resolved on all platforms, but we should wait for confirmation. |
Yep. All platforms please |
Triggered auto assignment to @srikarparsi, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
@dubielzyk-expensify we only scroll the header text in narrow layouts and apply the same approach for the categories page and other workspace pages. Screen.Recording.2024-09-23.at.5.56.25.PM.movAnd here, only small screen devices are selected.
|
@suneox With the selected proposal, the text will scroll in all cases. We only scroll the header text when we're in narrow layouts. |
@Nodebrute The expected behavior has been confirmed in this comment |
Yes, we must fix this on all platforms, but we only scroll the header text in narrow layouts. This is the same approach we use on other workspace pages. Can you check this video #49075 (comment) |
In the wider layout example is it possible to have the intro text scroll, but not the table header? |
@dubielzyk-expensify Could you provide more details? I don't fully understand what you're referring to. |
I can fix the 'Name-Status' header, but I’m unable to make the 'Get a better overview ...' text scrollable: Screen.Recording.2024-09-24.at.10.19.50.mov |
@dubielzyk-expensify I don't see any similarity between the workspace 'More Features' page and other pages like workspace categories, tags, or taxes. Therefore, there’s no reason to require us to apply the same behavior on the 'More Features' page as on those other pages. So I think we just need to apply the changes to all platforms as you mentioned above:
|
Yeah, I think for the pages with tables, let's just leave them as is for now and proceed with the original bug in this issue. It's still solvable by making a sticky header table when scrolling, but it requires more work |
@srikarparsi C+ has reviewed the proposal. All yours! |
Definitely agree with this so I agree with where you all landed. Make |
📣 @suneox 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app! |
📣 @truph01 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app! Offer link |
Checklist
|
@abekkala The PR has been deployed to production for a week, so we can proceed with the payment for this issue. |
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: 9.0.33-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: @dubielzyk-expensify
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1726115971343019
Action Performed:
Expected Result:
The intro text that says: "use the toggles below to enable..."
is fixed when you scroll the page
Actual Result:
Intro text should be attached to the content of the page and scroll with the rest of the cards. Also the padding on the right of the text missing. The text is too close to the edge:
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
CleanShot.2024-09-12.at.14.40.13.mp4
RPReplay_Final1726144041.MP4
Add any screenshot/video evidence
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @suneoxThe text was updated successfully, but these errors were encountered: