ANDROID-10730 Initial branch with nested scroll webview implementation #1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dpastor
commented
Aug 4, 2022
| @@ -0,0 +1,91 @@ | |||
| package com.telefonica.nestedscrollwebview | |||
There was a problem hiding this comment.
Most important class
dpastor
commented
Aug 4, 2022
| import android.view.ViewParent | ||
| import androidx.coordinatorlayout.widget.CoordinatorLayout | ||
|
|
||
| class CoordinatorLayoutChildHelper { |
There was a problem hiding this comment.
Workaround for --> https://issuetracker.google.com/issues/37093004
dpastor
commented
Aug 4, 2022
| import com.telefonica.nestedscrollwebview.helper.NestedScrollViewChild | ||
| import com.telefonica.nestedscrollwebview.helper.NestedScrollViewHelper | ||
|
|
||
| class NestedScrollWebView: WebView, NestedScrollViewChild { |
There was a problem hiding this comment.
Workaround to solve two main problems when using a webView inside a NestedScrollView:
- WebView height calculation.
- WebView must process its own scrolling from touch events.
dpastor
commented
Aug 4, 2022
Comment on lines
42
to
48
| /** | ||
| * This class extracts the following functionality from NestedScrollView class (Android Support Library Compat 1.8.0): | ||
| * * NestedScrollView.onTouchEvent(@NonNull MotionEvent ev) implementation | ||
| * * NestedScrollView.computeScroll() implementation | ||
| * * NestedScrollView NestedScrollingChild3 interface implementation | ||
| * Views can delegate nested scrolling logic to this class, by implementing NestedChildView interface. | ||
| */ |
There was a problem hiding this comment.
Explanation for this class. Idea of it is that implementation use composition instead of inheritance to apply this functionality. Also, it isolates support library original code from NestedScrollWebView implementation.
dpastor
commented
Aug 4, 2022
Comment on lines
+1
to
+15
| /* | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| * | ||
| * Based on the implementation of NestedScrollView from The Android Open Source Project | ||
| */ |
There was a problem hiding this comment.
We should include license file, as the code is from app compat library.
Comment on lines
47
to
62
| override fun onCreateOptionsMenu(menu: Menu): Boolean { | ||
| // Inflate the menu; this adds items to the action bar if it is present. | ||
| menuInflater.inflate(R.menu.menu_scrolling, menu) | ||
| return true | ||
| } | ||
|
|
||
| override fun onOptionsItemSelected(item: MenuItem): Boolean { | ||
| // Handle action bar item clicks here. The action bar will | ||
| // automatically handle clicks on the Home/Up button, so long | ||
| // as you specify a parent activity in AndroidManifest.xml. | ||
|
|
||
| return when (item.itemId) { | ||
| R.id.action_settings -> true | ||
| else -> super.onOptionsItemSelected(item) | ||
| } | ||
| } |
There was a problem hiding this comment.
Adapted on last commits to toogle between the different behaviours
| import com.telefonica.nestedscrollwebview.helper.NestedScrollViewChild | ||
| import com.telefonica.nestedscrollwebview.helper.NestedScrollViewHelper | ||
|
|
||
| class NestedScrollWebView: WebView, NestedScrollViewChild { |
There was a problem hiding this comment.
I think it should be open, so it can be extended, also I would open the methods, as well
Comment on lines
76
to
90
| override fun computeHorizontalScrollRange(): Int = | ||
| super.computeHorizontalScrollRange() | ||
|
|
||
| override fun computeHorizontalScrollExtent(): Int = | ||
| super.computeHorizontalScrollExtent() | ||
|
|
||
| override fun computeVerticalScrollRange(): Int = | ||
| super.computeVerticalScrollRange() | ||
|
|
||
| override fun computeVerticalScrollExtent(): Int = | ||
| super.computeVerticalScrollExtent() | ||
|
|
||
| override fun onOverScrolled(scrollX: Int, scrollY: Int, clampedX: Boolean, clampedY: Boolean) { | ||
| super.onOverScrolled(scrollX, scrollY, clampedX, clampedY) | ||
| } |
There was a problem hiding this comment.
All these have protected visibility in webView class, so it is just to expose visibility to the NesteScrollViewHelper.
…erface + Cleanup
…hing behaviour optional and disabled by default
…tching enabling
|
Re-requesting review with last changes on configuration and sample app |
gmerinojimenez
approved these changes
Aug 8, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Initial commit for the repo.
It contains a library module and a simple sample app to check the implementation.
There's still some formalities to do, as documentation and prepare the artifact publishing.
Please check this firstly so I can get feedback on it to prepare the repo completely.