Closed lanitochka17 closed 10 months ago
Triggered auto assignment to @mallenexpensify (Bug
), see https://stackoverflow.com/c/expensify/questions/14418 for more details.
Platforms
in OP are ✅):wave: Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open StagingDeployCash
deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:
Triggered auto assignment to @pecanoro (Engineering
), see https://stackoverflow.com/c/expensify/questions/4319 for more details.
On distance request page, map preview is not extended to full view on RHP
In the newly created DraggableList component, we wrap all contents inside ScrollView
as you can see
https://github.com/Expensify/App/blob/eb5e1e82e1745d13d06d32f681c5fe81efb0d697/src/components/DraggableList/index.tsx#L73
ScrollView
has style flex1
so that it covers all possible space, but this ScrollView
component has a content container in it. Please refer https://reactnative.dev/docs/scrollview#contentcontainerstyle. We doesn't set the style of this content container so the container doesn't cover the possible space.
This is the root cause
Set the contentContainerStyle
of ScrollView to styles.flex1
This works as expected
What will be max height on mobile?
@shawnborton since you were involved in the Distance Request screen design,
should the map be stretched out to fill the remaining space or the current heigh is okay?
Please check the hight here
I think min-height: 300, max-height: 500
I was also thinking to add flex to contentContainerStyle
but web and native are implemented differently so it won't work on native.
We should confirm if map should be stretched on native as well.
This is a new feature/ change and the behaviour is not blocking user so I am removing the blocker label and the original PR author can address the changes based on design feedback.
Let me know if you disagree about the blocker status. thanks!
I was also thinking to add flex to
contentContainerStyle
but web and native are implemented differently so it won't work on native. We should confirm if map should be stretched on native as well.
This one solves the issue. Preparing PR with that change.
The mentioned change doesn't solve the issue on natives
For native platforms, we need to change the DraggableList as follows
return (
<DraggableFlatList
ref={ref}
contentContainerStyle={styles.flex1}
containerStyle={styles.flex1}
ListFooterComponentStyle={styles.flex1}
// eslint-disable-next-line react/jsx-props-no-spreading
{...viewProps}
/>
);
should the map be stretched out to fill the remaining space or the current heigh is okay?
Yes, it should fill the available space. I feel like we have multiple threads/places where we've given feedback about the correct design and behavior of this page. @trjExpensify or @JmillsExpensify can you help shed some light on that here?
Reminder: please add distance related issues to the project. It helps us keep track of them.
Yes, it should fill the available space. I feel like we have multiple threads/places where we've given feedback about the correct design and behavior of this page. @trjExpensify or @JmillsExpensify can you help shed some light on that here?
It looks like this was a regression found when executing this PR, which is linked to the issue that references the main discussion on deciding the design of this: https://expensify.slack.com/archives/C05DWUDHVK7/p1694509495995009
While there are a healthy number of PR reviews on that PR, and it was shared in the thread, there wasn't a review requested from someone on the design team. So I think this is just another reminder to make sure that happens, and this would likely have been caught. CC: @hayata-suenaga
@pecanoro I can take this over if you like since it's a distance requests issue.
@neil-marcellini Nah, it's fine, it seems like an easy issue to review.
@neil-marcellini Nah, it's fine, it seems like an easy issue to review.
Reviewing
label has been removed, please complete the "BugZero Checklist".
The solution for this issue has been :rocket: deployed to production :rocket: in version 1.3.88-11 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:
If no regressions arise, payment will be issued on 2023-10-30. :confetti_ball:
After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.
As a reminder, here are the bonuses/penalties that should be applied for any External 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:
Ah we can close this one since it was a regression and it was fixed by the original person
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Issue found when executing PR https://github.com/Expensify/App/pull/26307
Action Performed:
Expected Result:
Map preview is extended to full view on RHP
Actual Result:
Map preview is not extended to full view on RHP
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Version Number: 1.3.78-0
Reproducible in staging?: Yes
Reproducible in production?: No
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://github.com/Expensify/App/assets/78819774/40cc133f-4746-480e-bc41-018a3b1c477c
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:
View all open jobs on GitHub