-
Notifications
You must be signed in to change notification settings - Fork 3.9k
fix loading indicator when the app is loading #56314
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
Changes from all commits
3048d37
d430a4a
9a04216
17f51a3
04dc31f
1118aa6
bf265ab
2e38961
03ca37b
80e7b6f
4289617
27ca1fd
9097e7f
8e2ddba
15fdd11
b9b4852
181d081
584d56a
c7be3bd
1cf8145
a7ad6ed
32e5e85
1886298
f963599
90391f3
7401b9d
7ad6120
d109fc1
833bca5
5ddd049
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3743,7 +3743,7 @@ const styles = (theme: ThemeColors) => | |
| }, | ||
|
|
||
| narrowSearchHeaderStyle: { | ||
| paddingTop: 1, | ||
| paddingTop: 12, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure why we adjusted this but it's causing this: Screen.Recording.2025-02-21.at.10.59.16.at.night.movComponent appears to be cut off. This is before: cc @shawnborton if this is something we'd want to fix.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @getusha What is the bug here, the empty state component is scrollable, and when the height is not enough, it will appear like this. Without this padding, you can also see the cut off in the device with small height.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catches on all of that, we want to fix these things for sure. For the new search input on desktop, maybe we can put the loading bar underneath it in the gap between the search input and the tabs?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or actually I'd be curious to see it at the very very top of the view even, on the top edge of the screen. Let's see what @Expensify/design thinks about that one since we don't have a standard page header on this particular page.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd definitely be down to see it at the very top.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I could have sworn we were showing it there for Reports/Search previously though? Otherwise yeah, we only show it in the MCP (lol) on mobile when a chat is loading I think...
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel like we've gone around a few places here, but I like the video above where it shows only in LHN area 👍 I see what you wanted with the top there Shawn, but I feel like I'd rather then explore one of the first explorations I did where it was always at the top. But then again, I think that's just a completely different thing. Would prefer it to stay in a consistent place as much as possible
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Totally agree, I think keeping the loading bar in the LHN for reports makes sense and I love that it translates well to mobile placement too.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
@shawnborton It's worked well, see this comment #56314 (comment).
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree! Let's start final reviewing this one cc @getusha |
||
| backgroundColor: theme.appBG, | ||
| flex: 1, | ||
| }, | ||
|
|
@@ -5415,7 +5415,8 @@ const styles = (theme: ThemeColors) => | |
| width: '100%', | ||
| backgroundColor: theme.transparent, | ||
| overflow: 'hidden', | ||
| marginBottom: -1, | ||
| position: 'absolute', | ||
| bottom: -1, | ||
| }, | ||
|
|
||
| progressBar: { | ||
|
|
||



Uh oh!
There was an error while loading. Please reload this page.