-
Notifications
You must be signed in to change notification settings - Fork 221
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
Fixing readme loading: Using webview #718
base: master
Are you sure you want to change the base?
Fixing readme loading: Using webview #718
Conversation
356a68e
to
ac066fe
Compare
#661 already is fixed, though. |
What I meant is that in one of last discussion I've been told that WebView has been avoided to load the Readme and a custom solution had been prefer because of scrolling issues of nested views ... Wouldn't it be nice to delegate all this custom implemented behaviour to WebView ? |
Yes, among other things, in particular styling. For comparison, two screenshots of the Octodroid repo being displayed, with current solution and WebView: The WebView styling simply doesn't fit in as well, at least not without applying a lot of CSS magic.
Maybe, but we'll need to keep the custom handling for issue/PR comments anyway. |
Why is that? I obviously considered possibility of adjusting the solution, for the moment it was just trying to check if it was possible. Shouldn't the WebView handle horizontal scrolling for itself while swipe to move to other tabs should be provided by the activity? |
Because having one WebView per issue comment view totally destroys performance. Yes, we've tried.
Yes, but different horizontal swipe behaviour depending on where you swipe is bad from a UX perspective - it's just confusing. |
Yep I can imagine ...
Isn't this what the app currently provides in RepositoryFragment? |
No, RepositoryFragment doesn't scroll horizontally. |
Generally yes, but if the target area is easily distinguishable (like code blocks) then there should be no confusion. @MattiaPrimavera Did you test how horizontal scrolling works in code blocks with long text. From my experience WebView doesn't work well with ViewPager. (But maybe they fixed it) Right now this pull request is a no go. Not before the WebView gets styling rules to look similar to what TextView based readme looks like. And I think we actually need these styling rules for the preview in comment editor so pull request improving that would be great on its own. EDIT: In my opinion WebView based readme would be benefit just because of these scrollable code blocks. I really dislike how they are wrapped in TextView and lots of Readme files contain examples in code blocks. |
@Tunous well I tried scrolling horizontally on a long text, if that's what you mean yes ... it looked like working
Totally agreed, the intention was not replacing with something uglier :P of course, but trying to see if a WebView could be used instead ;)
|
I tried to use a WebView for solving #661
It seems like it does work, no scrolling issues, since vertical one is disabled (still provided by the parent view) while the horizontal scroll stays enabled on the WebView.
This leads to :