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

zim_manager: Replace the spinner with a SwipeRefreshLayout #297

Merged
merged 10 commits into from Nov 30, 2017

Conversation

Projects
None yet
3 participants
@divadsn
Collaborator

divadsn commented Nov 30, 2017

Fixes #162

Also found a bug where the addBook method in ZimFileSelectFragment wasn't checking if the file list is empty after downloading a new book, see commit ca6b027.

divadsn added some commits Nov 29, 2017

zim_manager: Replace RelativeLayout with SwipeRefreshLayout
Signed-off-by: David Sn <divad.nnamtdeis@gmail.com>
Replace refresh button with swipe gesture
Signed-off-by: David Sn <divad.nnamtdeis@gmail.com>
@Albert221

This comment has been minimized.

Albert221 commented on df3dcc5 Nov 29, 2017

Wouldn't it be better to have SwipeRefreshLayout in zim_manager.xml?

This comment has been minimized.

Owner

divadsn replied Nov 29, 2017

No, as zim_list.xml is being used for llLayout in ZimFileSelectFragment at line 105.

This comment has been minimized.

Owner

divadsn replied Nov 29, 2017

See commit 6d0fbb8 for the actual use ;)

divadsn added some commits Nov 29, 2017

Remove "progress bar" in favor of swipe animation
Signed-off-by: David Sn <divad.nnamtdeis@gmail.com>
Fix not refreshing list if user has swiped to refresh
Signed-off-by: David Sn <divad.nnamtdeis@gmail.com>
Fix "no files found" after downloading a file
Signed-off-by: David Sn <divad.nnamtdeis@gmail.com>

@divadsn divadsn changed the title from zim_manager: Replace the spinner with a SwipeRefreshLayout to zim_manager: Replace the spinner with a SwipeRefreshLayout (WIP) Nov 30, 2017

divadsn added some commits Nov 30, 2017

Remove menu_rescan_fs from tests and replace click with swipeDown
Signed-off-by: David Sn <divad.nnamtdeis@gmail.com>

@divadsn divadsn force-pushed the divadsn:swiperefresh-layout branch from 26ccc33 to 52c142d Nov 30, 2017

@@ -1,12 +0,0 @@
<?xml version="1.0" encoding="utf-8"?>

This comment has been minimized.

@divadsn

divadsn Nov 30, 2017

Collaborator

This file was marked by AS as redundant.

Add refresh on swipe for online content
Signed-off-by: David Sn <divad.nnamtdeis@gmail.com>

@divadsn divadsn changed the title from zim_manager: Replace the spinner with a SwipeRefreshLayout (WIP) to zim_manager: Replace the spinner with a SwipeRefreshLayout Nov 30, 2017

@divadsn

This comment has been minimized.

Collaborator

divadsn commented Nov 30, 2017

Instead of showing the no network connection text when a refresh has failed, I decided to show a toast as the user should be still able to browse the list in it's unrefreshed state.

TestingUtils.unbindResource(LibraryFragment.class);
}
public void noNetworkConnection() {
displayNoNetworkConnection();
}
public void refreshFragment() {

This comment has been minimized.

@divadsn

divadsn Nov 30, 2017

Collaborator

I'm not 100% happy with the refreshFragment method, but I couldn't find a better way to trigger a reload instead of waiting for the broadcast receiver to do it's job when the connection was available.

This comment has been minimized.

@mhutti1

mhutti1 Nov 30, 2017

Member

This looks good to me.

Disable swipe to refresh if no network connection
Prevents the user from requesting a refresh when the list isn't loaded yet.

@mhutti1 mhutti1 self-assigned this Nov 30, 2017

@@ -103,8 +104,8 @@ public void downloadTest() {
allOf(withText("Device"), isDisplayed()));
appCompatTextView3.perform(click());
onView(withId(R.id.menu_rescan_fs))
.perform(click());
onView(withId(R.id.swiperefresh))

This comment has been minimized.

@mhutti1

mhutti1 Nov 30, 2017

Member

I am failing download and network tests with

android.support.test.espresso.AmbiguousViewMatcherException: 'with id: org.kiwix.kiwixmobile:id/swiperefresh' matches multiple views in the hierarchy. Problem views are marked with '****MATCHES****' below.

This comment has been minimized.

@divadsn

divadsn Nov 30, 2017

Collaborator

Hmm, so how is it supposed to work then? Isn't a swipeDown doing the action?

This comment has been minimized.

@mhutti1

mhutti1 Nov 30, 2017

Member

You have given 2 layouts the same ID. The test doesn't know which one to swipe on.

This comment has been minimized.

@divadsn

divadsn Nov 30, 2017

Collaborator

Aaa, now I get it, fix is coming. :)

@divadsn

This comment has been minimized.

Collaborator

divadsn commented Nov 30, 2017

Should be fine now, I also renamed it to the right id for the tests.

Btw, how can I do these tests in the future to solve those issues before?

Rename swiperefresh to seperate id for both tabs
Signed-off-by: David Sn <divad.nnamtdeis@gmail.com>

@divadsn divadsn force-pushed the divadsn:swiperefresh-layout branch from 41d0891 to 964a699 Nov 30, 2017

@mhutti1 mhutti1 merged commit 44a2e29 into kiwix:master Nov 30, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment