jenkins-bot has submitted this change and it was merged.
Change subject: Iron out issues in Read More component.
......................................................................
Iron out issues in Read More component.
- Dynamically size the component based on the number of page suggestions
retrieved.
- Fix layout issues / inconsistencies.
- Will follow up with patch to enable scrolling when swiping the list
items.
Change-Id: Ie5b02c0afca51f44ff444f1759847945fb72aba6
---
M wikipedia/res/layout/fragment_page.xml
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/views/ObservableWebView.java
4 files changed, 143 insertions(+), 74 deletions(-)
Approvals:
BearND: Looks good to me, approved
jenkins-bot: Verified
diff --git a/wikipedia/res/layout/fragment_page.xml
b/wikipedia/res/layout/fragment_page.xml
index 402ca01..362e285 100644
--- a/wikipedia/res/layout/fragment_page.xml
+++ b/wikipedia/res/layout/fragment_page.xml
@@ -75,14 +75,14 @@
</FrameLayout>
</FrameLayout>
- <FrameLayout
+ <LinearLayout
android:id="@+id/bottom_content_container"
android:layout_width="match_parent"
android:layout_height="wrap_content"
- android:orientation="vertical"
- android:background="?attr/page_background_color"
+ android:layout_marginBottom="-400dp"
android:layout_gravity="bottom"
- android:visibility="invisible">
+ android:orientation="vertical"
+ android:visibility="gone">
<LinearLayout
android:id="@+id/read_more_container"
android:layout_width="match_parent"
@@ -92,38 +92,35 @@
style="?android:textAppearanceLarge"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
+ android:paddingLeft="@dimen/activity_horizontal_margin"
+
android:paddingRight="@dimen/activity_horizontal_margin"
android:paddingBottom="16dp"
- android:paddingLeft="16dp"
- android:paddingRight="16dp"
android:fontFamily="serif"
android:textSize="24sp"
- android:text="@string/read_more_section"
- />
+ android:text="@string/read_more_section"/>
<ListView
android:id="@+id/read_more_list"
android:layout_width="match_parent"
- android:layout_height="320dp">
- </ListView>
+ android:layout_height="0dp"/>
</LinearLayout>
<LinearLayout
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:orientation="vertical"
- android:layout_gravity="bottom"
+ android:paddingBottom="16dp"
android:background="?attr/subtle_gray_color">
<View
android:layout_width="match_parent"
android:layout_height="8dp"
- android:layout_gravity="top"
android:background="@drawable/toolbar_bottom_shadow"/>
<TextView
android:id="@+id/page_last_updated_text"
android:layout_width="match_parent"
android:layout_height="wrap_content"
+ android:paddingTop="16dp"
style="?android:textAppearanceSmall"
android:textColorLink="?attr/link_color"
- android:paddingTop="16dp"
android:gravity="center"/>
<TextView
android:id="@+id/page_license_text"
@@ -131,10 +128,9 @@
android:layout_height="wrap_content"
style="?android:textAppearanceSmall"
android:textColorLink="?attr/link_color"
- android:paddingBottom="16dp"
android:gravity="center"/>
</LinearLayout>
- </FrameLayout>
+ </LinearLayout>
</FrameLayout>
diff --git
a/wikipedia/src/main/java/org/wikipedia/page/PageViewFragmentInternal.java
b/wikipedia/src/main/java/org/wikipedia/page/PageViewFragmentInternal.java
index b6156fd..522c86b 100644
--- a/wikipedia/src/main/java/org/wikipedia/page/PageViewFragmentInternal.java
+++ b/wikipedia/src/main/java/org/wikipedia/page/PageViewFragmentInternal.java
@@ -113,6 +113,7 @@
private PageTitle titleOriginal;
private ViewGroup imagesContainer;
private LeadImagesHandler leadImagesHandler;
+ private BottomContentHandler bottomContentHandler;
private ObservableWebView webView;
private SwipeRefreshLayoutWithScroll refreshView;
private View networkError;
@@ -414,6 +415,11 @@
imagesContainer = (ViewGroup)
parentFragment.getView().findViewById(R.id.page_images_container);
leadImagesHandler = new LeadImagesHandler(parentFragment, bridge,
webView, imagesContainer);
+ bottomContentHandler = new BottomContentHandler(parentFragment, bridge,
+ webView, linkHandler,
+ (ViewGroup)
parentFragment.getView().findViewById(R.id.bottom_content_container),
+ title);
+
//is this page in cache??
if (PAGE_CACHE.has(titleOriginal)) {
Log.d(TAG, "Using page from cache: " +
titleOriginal.getDisplayText());
@@ -481,11 +487,8 @@
// Do any other stuff that should happen upon page load
completion...
getActivity().updateProgressBar(false, true, 0);
- // create bottom content for this page...
- new BottomContentHandler(parentFragment, bridge, webView,
linkHandler,
- (ViewGroup)
parentFragment.getView().findViewById(R.id.bottom_content_container),
- title, page.getPageProperties().isMainPage());
-
+ // trigger layout of the bottom content
+ bottomContentHandler.beginLayout();
}
});
}
@@ -502,6 +505,8 @@
webView.setVisibility(View.GONE);
// and the lead image
leadImagesHandler.hide();
+ // and the bottom content
+ bottomContentHandler.hide();
// and reload the page...
setState(STATE_NO_FETCH);
@@ -888,6 +893,7 @@
private void showNetworkError() {
// Check for the source of the error and have different things turn up
leadImagesHandler.hide();
+ bottomContentHandler.hide();
webView.setVisibility(View.INVISIBLE);
ViewAnimations.fadeIn(networkError);
}
@@ -1004,6 +1010,8 @@
webView.setVisibility(View.GONE);
// and the lead image
leadImagesHandler.hide();
+ // and the bottom content
+ bottomContentHandler.hide();
curEntry = new HistoryEntry(title, HistoryEntry.SOURCE_HISTORY);
setState(STATE_NO_FETCH);
performActionForState(state);
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 86b1180..2ab97a7 100644
---
a/wikipedia/src/main/java/org/wikipedia/page/bottomcontent/BottomContentHandler.java
+++
b/wikipedia/src/main/java/org/wikipedia/page/bottomcontent/BottomContentHandler.java
@@ -1,9 +1,8 @@
package org.wikipedia.page.bottomcontent;
-import android.graphics.Point;
-import android.os.Build;
import android.text.Html;
import android.text.TextUtils;
+import android.util.Log;
import android.view.LayoutInflater;
import android.view.View;
import android.view.ViewGroup;
@@ -12,6 +11,7 @@
import android.widget.BaseAdapter;
import android.widget.FrameLayout;
import android.widget.ImageView;
+import android.widget.ListAdapter;
import android.widget.ListView;
import android.widget.TextView;
@@ -40,7 +40,9 @@
import java.util.List;
import java.util.Map;
-public class BottomContentHandler implements
ObservableWebView.OnScrollChangeListener {
+public class BottomContentHandler implements
ObservableWebView.OnScrollChangeListener,
+ ObservableWebView.OnContentHeightChangedListener {
+ private static final String TAG = "BottomContentHandler";
private final PageViewFragment parentFragment;
private final CommunicationBridge bridge;
private final WebView webView;
@@ -48,17 +50,20 @@
private final PageTitle pageTitle;
private final PageActivity activity;
private final WikipediaApp app;
-
- private int displayHeight;
private float displayDensity;
+ private boolean firstTimeShown = false;
private View bottomContentContainer;
private TextView pageLastUpdatedText;
private TextView pageLicenseText;
+ private ListView readMoreList;
+
+ private SuggestedPagesFunnel funnel;
+ private FullSearchArticlesTask.FullSearchResults readMoreItems;
public BottomContentHandler(final PageViewFragment parentFragment,
CommunicationBridge bridge,
- ObservableWebView webview, LinkHandler
linkHandler, ViewGroup hidingView,
- PageTitle pageTitle, boolean isMainPage) {
+ ObservableWebView webview, LinkHandler
linkHandler,
+ ViewGroup hidingView, PageTitle pageTitle) {
this.parentFragment = parentFragment;
this.bridge = bridge;
this.webView = webview;
@@ -70,77 +75,115 @@
bottomContentContainer = hidingView;
webview.addOnScrollChangeListener(this);
+ webview.addOnContentHeightChangedListener(this);
pageLastUpdatedText =
(TextView)bottomContentContainer.findViewById(R.id.page_last_updated_text);
pageLicenseText =
(TextView)bottomContentContainer.findViewById(R.id.page_license_text);
+ readMoreList =
(ListView)bottomContentContainer.findViewById(R.id.read_more_list);
+
+ funnel = new SuggestedPagesFunnel(app, pageTitle.getSite());
// preload the display density, since it will be used in a lot of
places
displayDensity =
parentFragment.getResources().getDisplayMetrics().density;
-
- // get the screen height, using correct methods for different API
versions
- if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.HONEYCOMB_MR2) {
- Point size = new Point();
-
parentFragment.getActivity().getWindowManager().getDefaultDisplay().getSize(size);
- displayHeight = (int)(size.y / displayDensity);
- } else {
- displayHeight = (int)(parentFragment.getActivity()
- .getWindowManager().getDefaultDisplay().getHeight() /
displayDensity);
- }
-
- if (isMainPage) {
-
bottomContentContainer.findViewById(R.id.read_more_container).setVisibility(View.GONE);
- } else {
- requestReadMoreItems(activity.getLayoutInflater());
- }
-
- setupAttribution();
-
- // give it a chance to redraw, and then see if it fits
- bottomContentContainer.post(new Runnable() {
- @Override
- public void run() {
- if (!parentFragment.isAdded()) {
- return;
- }
- layoutContent();
- }
- });
+ // hide ourselves by default
+ hide();
}
@Override
public void onScrollChanged(int oldScrollY, int scrollY) {
+ if (bottomContentContainer.getVisibility() == View.GONE) {
+ return;
+ }
int contentHeight = (int)(webView.getContentHeight() * displayDensity);
- int screenHeight = (int)(displayHeight * displayDensity);
- final int bottomOffsetExtra = 25;
- int bottomOffset = contentHeight - scrollY - screenHeight
- + (int)(bottomOffsetExtra * displayDensity);
+ int bottomOffset = contentHeight - scrollY - webView.getHeight();
int bottomHeight = bottomContentContainer.getHeight();
FrameLayout.LayoutParams params = (FrameLayout.LayoutParams)
bottomContentContainer.getLayoutParams();
if (bottomOffset > bottomHeight) {
if (params.bottomMargin != -bottomHeight) {
params.bottomMargin = -bottomHeight;
bottomContentContainer.setLayoutParams(params);
+ bottomContentContainer.setVisibility(View.INVISIBLE);
}
} else {
params.bottomMargin = -bottomOffset;
bottomContentContainer.setLayoutParams(params);
+ if (bottomContentContainer.getVisibility() != View.VISIBLE) {
+ bottomContentContainer.setVisibility(View.VISIBLE);
+ }
+ if (!firstTimeShown && readMoreItems != null) {
+ firstTimeShown = true;
+ funnel.logSuggestionsShown(pageTitle,
readMoreItems.getPageTitles());
+ }
+ }
+ }
+
+ @Override
+ public void onContentHeightChanged(int contentHeight) {
+ if (bottomContentContainer.getVisibility() != View.VISIBLE) {
+ return;
+ }
+ // trigger a manual scroll event to update our position
+ onScrollChanged(webView.getScrollY(), webView.getScrollY());
+ }
+
+ /**
+ * Hide the bottom content entirely.
+ * It can only be shown again by calling layoutContent()
+ */
+ public void hide() {
+ bottomContentContainer.setVisibility(View.GONE);
+ }
+
+ public void beginLayout() {
+ setupAttribution();
+ if
(parentFragment.getFragment().getPage().getPageProperties().isMainPage()) {
+
bottomContentContainer.findViewById(R.id.read_more_container).setVisibility(View.GONE);
+ layoutContent();
+ } else {
+ requestReadMoreItems(activity.getLayoutInflater());
}
}
private void layoutContent() {
+ if (!parentFragment.isAdded()) {
+ return;
+ }
+ bottomContentContainer.setVisibility(View.INVISIBLE);
+ // keep trying until our layout has a height...
+ if (bottomContentContainer.getHeight() == 0) {
+ final int postDelay = 50;
+ bottomContentContainer.postDelayed(new Runnable() {
+ @Override
+ public void run() {
+ layoutContent();
+ }
+ }, postDelay);
+ return;
+ }
+
+ // calculate the height of the listview, based on the number of items
inside it.
+ ListAdapter adapter = readMoreList.getAdapter();
+ int listHeight = 0;
+ if (adapter != null && adapter.getCount() > 0) {
+ ViewGroup.LayoutParams params = readMoreList.getLayoutParams();
+ final int itemHeight =
(int)parentFragment.getResources().getDimension(R.dimen.defaultListItemSize);
+ listHeight = adapter.getCount() * itemHeight
+ + (readMoreList.getDividerHeight() * (adapter.getCount() -
1));
+ params.height = listHeight;
+ readMoreList.setLayoutParams(params);
+ }
+
// pad the bottom of the webview, to make room for ourselves
+ int totalHeight = bottomContentContainer.getHeight() + listHeight;
JSONObject payload = new JSONObject();
try {
- payload.put("paddingBottom",
(int)(bottomContentContainer.getHeight() / displayDensity));
+ payload.put("paddingBottom", (int)(totalHeight / displayDensity));
} catch (JSONException e) {
throw new RuntimeException(e);
}
bridge.sendMessage("setPaddingBottom", payload);
-
- // and make it visible!
- bottomContentContainer.setVisibility(View.VISIBLE);
- // trigger a manual scroll event to update our position
- onScrollChanged(webView.getScrollY(), webView.getScrollY());
+ // ^ sending the padding event will guarantee a ContentHeightChanged
event to be triggered,
+ // which will update our margin based on the scroll offset, so we
don't need to do it here.
}
private void setupAttribution() {
@@ -160,23 +203,28 @@
pageTitle.getPrefixedText()) {
@Override
public void onFinish(FullSearchResults results) {
- setupReadMoreSection(bottomContentContainer, layoutInflater,
results);
+ readMoreItems = results;
+ if (readMoreItems.getResults().size() > 0) {
+ setupReadMoreSection(layoutInflater, readMoreItems);
+ }
+ layoutContent();
}
@Override
public void onCatch(Throwable caught) {
- super.onCatch(caught);
+ // Read More titles are expendable.
+ Log.w(TAG, "Error while fetching Read More titles.", caught);
+ // but lay out the bottom content anyway:
+ layoutContent();
}
}.execute();
}
- private void setupReadMoreSection(View parentView, LayoutInflater
layoutInflater,
+ private void setupReadMoreSection(LayoutInflater layoutInflater,
final
FullSearchArticlesTask.FullSearchResults results) {
- final SuggestedPagesFunnel funnel = new SuggestedPagesFunnel(app,
pageTitle.getSite());
final ReadMoreAdapter adapter = new ReadMoreAdapter(layoutInflater,
results.getResults());
- ListView list = (ListView)
parentView.findViewById(R.id.read_more_list);
- list.setAdapter(adapter);
- list.setOnItemClickListener(new AdapterView.OnItemClickListener() {
+ readMoreList.setAdapter(adapter);
+ readMoreList.setOnItemClickListener(new
AdapterView.OnItemClickListener() {
@Override
public void onItemClick(AdapterView<?> parent, View view, int
position, long id) {
PageTitle title = (PageTitle) adapter.getItem(position);
diff --git a/wikipedia/src/main/java/org/wikipedia/views/ObservableWebView.java
b/wikipedia/src/main/java/org/wikipedia/views/ObservableWebView.java
index cb5c118..817e9d8 100644
--- a/wikipedia/src/main/java/org/wikipedia/views/ObservableWebView.java
+++ b/wikipedia/src/main/java/org/wikipedia/views/ObservableWebView.java
@@ -14,6 +14,9 @@
private List<OnScrollChangeListener> onScrollChangeListeners;
private List<OnDownMotionEventListener> onDownMotionEventListeners;
private List<OnUpOrCancelMotionEventListener>
onUpOrCancelMotionEventListeners;
+ private List<OnContentHeightChangedListener>
onContentHeightChangedListeners;
+
+ private int contentHeight = 0;
public void addOnScrollChangeListener(OnScrollChangeListener
onScrollChangeListener) {
onScrollChangeListeners.add(onScrollChangeListener);
@@ -27,6 +30,10 @@
onUpOrCancelMotionEventListeners.add(onUpOrCancelMotionEventListener);
}
+ public void
addOnContentHeightChangedListener(OnContentHeightChangedListener
onContentHeightChangedListener) {
+ onContentHeightChangedListeners.add(onContentHeightChangedListener);
+ }
+
public interface OnScrollChangeListener {
void onScrollChanged(int oldScrollY, int scrollY);
}
@@ -37,6 +44,10 @@
public interface OnUpOrCancelMotionEventListener {
void onUpOrCancelMotionEvent();
+ }
+
+ public interface OnContentHeightChangedListener {
+ void onContentHeightChanged(int contentHeight);
}
public ObservableWebView(Context context) {
@@ -63,6 +74,7 @@
onScrollChangeListeners = new ArrayList<OnScrollChangeListener>();
onDownMotionEventListeners = new
ArrayList<OnDownMotionEventListener>();
onUpOrCancelMotionEventListeners = new
ArrayList<OnUpOrCancelMotionEventListener>();
+ onContentHeightChangedListeners = new
ArrayList<OnContentHeightChangedListener>();
}
@Override
@@ -102,7 +114,12 @@
if (isInEditMode()) {
return;
}
+ if (contentHeight != getContentHeight()) {
+ contentHeight = getContentHeight();
+ for (OnContentHeightChangedListener listener :
onContentHeightChangedListeners) {
+ listener.onContentHeightChanged(contentHeight);
+ }
+ }
WikipediaApp.getInstance().getBus().post(new WebViewInvalidateEvent());
}
-
}
--
To view, visit https://gerrit.wikimedia.org/r/174995
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie5b02c0afca51f44ff444f1759847945fb72aba6
Gerrit-PatchSet: 6
Gerrit-Project: apps/android/wikipedia
Gerrit-Branch: master
Gerrit-Owner: Dbrant <[email protected]>
Gerrit-Reviewer: BearND <[email protected]>
Gerrit-Reviewer: Brion VIBBER <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits