Skip to content

Fix scrolling on mobile#107

Merged
tgolen merged 5 commits into
masterfrom
andrew-scroll
Aug 12, 2020
Merged

Fix scrolling on mobile#107
tgolen merged 5 commits into
masterfrom
andrew-scroll

Conversation

@AndrewGable

@AndrewGable AndrewGable commented Aug 12, 2020

Copy link
Copy Markdown
Contributor

Closes #78 and #77

@tgolen @shawnborton - I had to remove some code, but I verified this works on local mobile/web.

@AndrewGable AndrewGable self-assigned this Aug 12, 2020
Comment thread src/style/StyleSheet.js
sidebarFooterAvatar: {
backgroundColor: colors.text,
borderRadius: '50%',
borderRadius: 20,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shawnborton - borderRadius can only be a number, so had to divide height/width by 2 manually.

Comment thread src/style/StyleSheet.js
height: 40,
justifyContent: 'center',
textDecoration: 'none',
textDecorationLine: 'none',

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We were just missing Line on this one

Comment thread src/style/StyleSheet.js
Comment thread src/style/StyleSheet.js
borderTopWidth: 1,
borderTopColor: colors.border,
minHeight: '85',
minHeight: 85,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

String -> Number

Comment thread src/style/StyleSheet.js
@shawnborton

Copy link
Copy Markdown
Contributor

How do all of these changes look on web? Especially the ellipsis truncation changes?

@AndrewGable

AndrewGable commented Aug 12, 2020

Copy link
Copy Markdown
Contributor Author

Super long report titles don't look perfect.. But not sure we care if we know the report titles for the POC? Thoughts?

Fixed in db823f7

@AndrewGable

Copy link
Copy Markdown
Contributor Author

Here is how it looks with the latest changes:

Screen Shot 2020-08-12 at 11 14 46 AM

Screen Shot 2020-08-12 at 11 14 49 AM

Screen Shot 2020-08-12 at 11 14 55 AM

Screen Shot 2020-08-12 at 11 14 58 AM

@shawnborton

Copy link
Copy Markdown
Contributor

Sweet! Last thing - should we address the scroll bar issue you are having? I am curious why you don't get it on the main chat window but you have it on the list item container.

Looks like the main chat area has overflow-x: hidden but I didn't seem to find that as a valid prop type?

@AndrewGable

Copy link
Copy Markdown
Contributor Author

Ok I fixed the scroll bar issue 😻

Screen Shot 2020-08-12 at 11 40 40 AM

However, one thing to note.. I don't think it scrolls when it has more than a few reports (on this PR or master). We might need to change a View to a ScrollView somewhere if we want that

@shawnborton

Copy link
Copy Markdown
Contributor

Well as long as the overflow: auto is still applied, it should scroll right?

@AndrewGable

Copy link
Copy Markdown
Contributor Author

On web, yes, but on mobile it doesn't seem to scroll the same. Also for the POC we only fetch 10 reports on dev, so I don't know if it will ever be an issue.

@shawnborton

Copy link
Copy Markdown
Contributor

Ah, got it. Okay, sounds good to me then!

Comment thread src/page/HomePage/Report/ReportHistoryView.js Outdated
Co-authored-by: Tim Golen <tgolen@expensify.com>
@tgolen tgolen merged commit 3a200a7 into master Aug 12, 2020
@tgolen tgolen deleted the andrew-scroll branch August 12, 2020 19:11
mountiny pushed a commit that referenced this pull request Feb 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Scroll down to show new comment on Android

3 participants