Dbrant has uploaded a new change for review. https://gerrit.wikimedia.org/r/200166
Change subject: Fix a couple issues with single WebView. ...................................................................... Fix a couple issues with single WebView. - Properly remember the y-scroll offset of the previous page. - Improve our WebView's custom onClick handler so that it gets consumed properly if it's handled by the Lead Image or Bottom Content container. Change-Id: Iafcbfadb9fcfac6d86841e2094808adb6c662848 --- M wikipedia/src/main/java/org/wikipedia/page/PageViewFragmentInternal.java M wikipedia/src/main/java/org/wikipedia/page/bottomcontent/BottomContentHandler.java M wikipedia/src/main/java/org/wikipedia/page/bottomcontent/BottomContentHandlerOld.java M wikipedia/src/main/java/org/wikipedia/page/leadimages/LeadImagesHandler.java M wikipedia/src/main/java/org/wikipedia/views/ObservableWebView.java 5 files changed, 56 insertions(+), 29 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/apps/android/wikipedia refs/changes/66/200166/1 diff --git a/wikipedia/src/main/java/org/wikipedia/page/PageViewFragmentInternal.java b/wikipedia/src/main/java/org/wikipedia/page/PageViewFragmentInternal.java index 59274f2..07f11f0 100644 --- a/wikipedia/src/main/java/org/wikipedia/page/PageViewFragmentInternal.java +++ b/wikipedia/src/main/java/org/wikipedia/page/PageViewFragmentInternal.java @@ -173,7 +173,7 @@ try { JSONObject marginPayload = new JSONObject(); int margin = (int) (getResources().getDimension(R.dimen.activity_horizontal_margin) - / getResources().getDisplayMetrics().density); + / getResources().getDisplayMetrics().density); marginPayload.put("marginLeft", margin); marginPayload.put("marginRight", margin); bridge.sendMessage("setMargins", marginPayload); @@ -195,16 +195,16 @@ bridge.sendMessage("displayLeadSection", leadSectionPayload); Utils.setupDirectionality(title.getSite().getLanguage(), - Locale.getDefault().getLanguage(), bridge); + Locale.getDefault().getLanguage(), bridge); // Hide edit pencils if anon editing is disabled by remote killswitch or if this is a file page JSONObject miscPayload = new JSONObject(); boolean isAnonEditingDisabled = app.getRemoteConfig().getConfig() - .optBoolean("disableAnonEditing", false) - && !app.getUserInfoStorage().isLoggedIn(); + .optBoolean("disableAnonEditing", false) + && !app.getUserInfoStorage().isLoggedIn(); miscPayload.put("noedit", (isAnonEditingDisabled - || title.isFilePage() - || page.getPageProperties().isMainPage())); + || title.isFilePage() + || page.getPageProperties().isMainPage())); miscPayload.put("protect", !page.getPageProperties().canEdit()); bridge.sendMessage("setPageProtected", miscPayload); } catch (JSONException e) { @@ -222,8 +222,8 @@ private void displayNonLeadSection(int index) { ((PageActivity) getActivity()).updateProgressBar(true, false, - PageActivity.PROGRESS_BAR_MAX_VALUE / page - .getSections().size() * index); + PageActivity.PROGRESS_BAR_MAX_VALUE / page + .getSections().size() * index); try { JSONObject wrapper = new JSONObject(); @@ -232,10 +232,10 @@ wrapper.put("section", page.getSections().get(index).toJSON()); wrapper.put("index", index); if (sectionTargetFromIntent > 0 && sectionTargetFromIntent < page.getSections() - .size()) { + .size()) { //if we have a section to scroll to (from our Intent): wrapper.put("fragment", - page.getSections().get(sectionTargetFromIntent).getAnchor()); + page.getSections().get(sectionTargetFromIntent).getAnchor()); } else if (sectionTargetFromTitle != null) { //if we have a section to scroll to (from our PageTitle): wrapper.put("fragment", sectionTargetFromTitle); @@ -245,7 +245,7 @@ } //give it our expected scroll position, in case we need the page to be pre-scrolled upon loading. wrapper.put("scrollY", - (int) (stagedScrollY / getResources().getDisplayMetrics().density)); + (int) (stagedScrollY / getResources().getDisplayMetrics().density)); bridge.sendMessage("displaySection", wrapper); } catch (JSONException e) { //nope @@ -341,7 +341,7 @@ referenceDialog.dismiss(); } HistoryEntry historyEntry = new HistoryEntry(title, - HistoryEntry.SOURCE_INTERNAL_LINK); + HistoryEntry.SOURCE_INTERNAL_LINK); ((PageActivity) getActivity()).displayNewPage(title, historyEntry); } @@ -356,7 +356,7 @@ protected void onReferenceClicked(String refHtml) { if (!isAdded()) { Log.d("PageViewFragment", - "Detached from activity, so stopping reference click."); + "Detached from activity, so stopping reference click."); return; } @@ -407,7 +407,7 @@ imagesContainer = (ViewGroup) getView().findViewById(R.id.page_images_container); leadImagesHandler = new LeadImagesHandler(getActivity(), this, bridge, webView, - imagesContainer); + imagesContainer); searchBarHideHandler = ((PageActivity) getActivity()).getSearchBarHideHandler(); searchBarHideHandler.setScrollView(webView); @@ -415,12 +415,12 @@ // (and when ready to release to production) if (BottomContentHandler.useNewBottomContent(app)) { bottomContentHandler = new BottomContentHandler(this, bridge, webView, linkHandler, - (ViewGroup) getView().findViewById( - R.id.bottom_content_container)); + (ViewGroup) getView().findViewById( + R.id.bottom_content_container)); } else { bottomContentHandler = new BottomContentHandlerOld(this, bridge, webView, linkHandler, - (ViewGroup) getView().findViewById( - R.id.bottom_content_container)); + (ViewGroup) getView().findViewById( + R.id.bottom_content_container)); } pageSequenceNum = 0; @@ -484,10 +484,9 @@ return; } PageBackStackItem item = backStack.get(backStack.size() - 1); - // display the page based on the backstack item... - displayNewPage(item.getTitle(), item.getHistoryEntry(), true, false); - // and stage the scrollY position based on the backstack item. - stagedScrollY = item.getScrollY(); + // display the page based on the backstack item, stage the scrollY position based on + // the backstack item. + displayNewPage(item.getTitle(), item.getHistoryEntry(), true, false, item.getScrollY()); } /** @@ -503,6 +502,22 @@ */ public void displayNewPage(PageTitle title, HistoryEntry entry, boolean tryFromCache, boolean pushBackStack) { + displayNewPage(title, entry, tryFromCache, pushBackStack, 0); + } + + /** + * Load a new page into the WebView in this fragment. + * This shall be the single point of entry for loading content into the WebView, whether it's + * loading an entirely new page, refreshing the current page, retrying a failed network + * request, etc. + * @param title Title of the new page to load. + * @param entry HistoryEntry associated with the new page. + * @param tryFromCache Whether to try loading the page from cache (otherwise load directly + * from network). + * @param pushBackStack Whether to push the new page onto the backstack. + */ + public void displayNewPage(PageTitle title, HistoryEntry entry, boolean tryFromCache, + boolean pushBackStack, int stagedScrollY) { if (pushBackStack) { // update the topmost entry in the backstack, before we start overwriting things. updateBackStackItem(); @@ -537,6 +552,7 @@ // whatever we pass to this event will be passed back to us by the WebView! wrapper.put("sequence", pageSequenceNum); wrapper.put("tryFromCache", tryFromCache); + wrapper.put("stagedScrollY", stagedScrollY); bridge.sendMessage("beginNewPage", wrapper); } catch (JSONException e) { //nope @@ -544,9 +560,6 @@ } private void loadPageOnWebViewReady(boolean tryFromCache) { - // reset staged scroll position to the top. - stagedScrollY = 0; - // stage any section-specific link target from the title, since the title may be // replaced (normalized) sectionTargetFromTitle = title.getFragment(); @@ -646,6 +659,7 @@ if (messagePayload.getInt("sequence") != pageSequenceNum) { return; } + stagedScrollY = messagePayload.getInt("stagedScrollY"); loadPageOnWebViewReady(messagePayload.getBoolean("tryFromCache")); } catch (JSONException e) { //nope diff --git a/wikipedia/src/main/java/org/wikipedia/page/bottomcontent/BottomContentHandler.java b/wikipedia/src/main/java/org/wikipedia/page/bottomcontent/BottomContentHandler.java index f55caba..58fab03 100644 --- a/wikipedia/src/main/java/org/wikipedia/page/bottomcontent/BottomContentHandler.java +++ b/wikipedia/src/main/java/org/wikipedia/page/bottomcontent/BottomContentHandler.java @@ -91,7 +91,10 @@ webview.addOnClickListener(new ObservableWebView.OnClickListener() { @Override - public void onClick(float x, float y) { + public boolean onClick(float x, float y) { + if (bottomContentContainer.getVisibility() != View.VISIBLE) { + return false; + } // if the click event is within the area of the lead image, then the user // must have wanted to click on the lead image! int[] pos = new int[2]; @@ -102,6 +105,7 @@ activity.displayNewPage(title, historyEntry); funnel.logSuggestionClicked(pageTitle, readMoreItems.getPageTitles(), 0); } + return true; } }); @@ -176,6 +180,10 @@ } public void beginLayout() { + firstTimeShown = false; + image1.setImageDrawable(null); + image1.setVisibility(View.INVISIBLE); + imagePlaceholder.setVisibility(View.VISIBLE); setupAttribution(); if (parentFragment.getPage().couldHaveReadMoreSection()) { preRequestReadMoreItems(); diff --git a/wikipedia/src/main/java/org/wikipedia/page/bottomcontent/BottomContentHandlerOld.java b/wikipedia/src/main/java/org/wikipedia/page/bottomcontent/BottomContentHandlerOld.java index a7f6dcd..6e67ca4 100644 --- a/wikipedia/src/main/java/org/wikipedia/page/bottomcontent/BottomContentHandlerOld.java +++ b/wikipedia/src/main/java/org/wikipedia/page/bottomcontent/BottomContentHandlerOld.java @@ -189,6 +189,7 @@ } public void beginLayout() { + firstTimeShown = false; setupAttribution(); if (parentFragment.getPage().couldHaveReadMoreSection()) { preRequestReadMoreItems(activity.getLayoutInflater()); diff --git a/wikipedia/src/main/java/org/wikipedia/page/leadimages/LeadImagesHandler.java b/wikipedia/src/main/java/org/wikipedia/page/leadimages/LeadImagesHandler.java index 9a85d73..1d9fd4a 100644 --- a/wikipedia/src/main/java/org/wikipedia/page/leadimages/LeadImagesHandler.java +++ b/wikipedia/src/main/java/org/wikipedia/page/leadimages/LeadImagesHandler.java @@ -140,7 +140,7 @@ webview.addOnClickListener(new ObservableWebView.OnClickListener() { @Override - public void onClick(float x, float y) { + public boolean onClick(float x, float y) { // if the click event is within the area of the lead image, then the user // must have wanted to click on the lead image! if (leadImagesEnabled && y < imageContainer.getHeight() - webView.getScrollY()) { @@ -152,7 +152,9 @@ .getSite()); parentFragment.showImageGallery(imageTitle); } + return true; } + return false; } }); diff --git a/wikipedia/src/main/java/org/wikipedia/views/ObservableWebView.java b/wikipedia/src/main/java/org/wikipedia/views/ObservableWebView.java index ac48678..eec65e9 100644 --- a/wikipedia/src/main/java/org/wikipedia/views/ObservableWebView.java +++ b/wikipedia/src/main/java/org/wikipedia/views/ObservableWebView.java @@ -48,7 +48,7 @@ } public interface OnClickListener { - void onClick(float x, float y); + boolean onClick(float x, float y); } public interface OnScrollChangeListener { @@ -119,7 +119,9 @@ if (Math.abs(event.getX() - touchStartX) <= touchSlop && Math.abs(event.getY() - touchStartY) <= touchSlop) { for (OnClickListener listener : onClickListeners) { - listener.onClick(event.getX(), event.getY()); + if (listener.onClick(event.getX(), event.getY())) { + return true; + } } } case MotionEvent.ACTION_CANCEL: -- To view, visit https://gerrit.wikimedia.org/r/200166 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Iafcbfadb9fcfac6d86841e2094808adb6c662848 Gerrit-PatchSet: 1 Gerrit-Project: apps/android/wikipedia Gerrit-Branch: master Gerrit-Owner: Dbrant <dbr...@wikimedia.org> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits