feat: Show no internet error in search activity #592

liveHarshit · 2018-09-27T19:54:31Z

Fixes #283

Screenshots of Eventyay and Evantbrite :

liveHarshit · 2018-09-27T19:56:21Z

onClickListner{} fot TextView is not working when TextView is outside android.support.v4.widget.NestedScrollView . Please suggest me what should I do?

app/src/main/java/org/fossasia/openevent/general/search/SearchFragment.kt

dreadpool2 · 2018-09-27T19:59:54Z

app/src/main/res/values/strings.xml

    <string name="no_search_results">Hmmm, we\'\re not getting\nany results. Our bad - try\nanother search</string>
    <string name="event_details">Event details</string>
    <string name="see_maps">See maps</string>
+    <string name="error_message">There was an error. Tab here to try again.</string>


Check whether the message is similar in EventBrite

Yes, it is similar in the updated version of EventBrite.

Could you please attach a screenshot of how it looks?

Could you please attach a screenshot of how it looks?

Uploaded in PR description.

@liveHarshit "Tap" here to try again

@liveHarshit "Tap" here to try again

Updated.

app/src/main/java/org/fossasia/openevent/general/event/EventsFragment.kt

simarsingh24 · 2018-09-28T05:31:21Z

app/src/main/java/org/fossasia/openevent/general/search/SearchFragment.kt

        }
        searchView.setOnQueryTextListener(queryListener)
        rootView.fabSearch.setOnClickListener {
+            showNoInternetScreen(isNetworkConnected())


move logic to view model, see how its done in login fragment and login fragment vm

move logic to view model, see how its done in login fragment and login fragment vm

But in event fragment it in not done in view model for check network connection, so I did the same. Shold I change it?

app/src/main/java/org/fossasia/openevent/general/data/Network.kt

shikhart98

There is an XML content_no_internet. You can use that instead.

app/src/main/res/values/strings.xml

iamareebjamal

Move all logic to ViewModels of respective fragments

liveHarshit · 2018-09-29T08:30:32Z

@iamareebjamal Updated.

iamareebjamal · 2018-09-29T14:06:03Z

app/src/main/java/org/fossasia/openevent/general/event/EventsFragment.kt

        }

-        showNoInternetScreen(isNetworkConnected())
+        eventsViewModel.showNoInternetScreen(rootView)


View should not be passed to ViewModel

iamareebjamal · 2018-09-29T14:06:20Z

app/src/main/java/org/fossasia/openevent/general/event/EventsViewModel.kt


 import android.arch.lifecycle.MutableLiveData
 import android.arch.lifecycle.ViewModel
+import android.view.View


No android imports in ViewModel

iamareebjamal · 2018-09-29T14:06:28Z

app/src/main/java/org/fossasia/openevent/general/event/EventsViewModel.kt

 import io.reactivex.disposables.CompositeDisposable
 import io.reactivex.schedulers.Schedulers
+import kotlinx.android.synthetic.main.content_no_internet.view.*
+import kotlinx.android.synthetic.main.fragment_events.view.*


Same as above

iamareebjamal · 2018-09-29T14:06:58Z

app/src/main/java/org/fossasia/openevent/general/event/EventsViewModel.kt


    private val compositeDisposable = CompositeDisposable()
    private val tokenKey = "LOCATION"
+    private val networkData = Network()


Inject instead of initializing

iamareebjamal · 2018-09-29T14:08:29Z

app/src/main/java/org/fossasia/openevent/general/search/SearchViewModel.kt

 import io.reactivex.android.schedulers.AndroidSchedulers
 import io.reactivex.disposables.CompositeDisposable
 import io.reactivex.schedulers.Schedulers
+import kotlinx.android.synthetic.main.fragment_search.view.*


Same as above

iamareebjamal · 2018-09-29T14:08:37Z

app/src/main/java/org/fossasia/openevent/general/search/SearchViewModel.kt

    val progress = MutableLiveData<Boolean>()
    val events = MutableLiveData<List<Event>>()
    val error = MutableLiveData<String>()
+    private val networkData = Network()


iamareebjamal · 2018-09-29T14:09:22Z

app/src/main/res/values/dimens.xml

    <dimen name="layout_margin_moderate">10dp</dimen>
    <dimen name="layout_margin_large">16dp</dimen>
    <dimen name="layout_margin_extra_large">32dp</dimen>
+    <dimen name="layout_margin_very_large">56dp</dimen>


very < extra

Can I change extra large 32dp to very large line 19 or take extra extra large for 56dp?

liveHarshit · 2018-09-29T15:25:06Z

@iamareebjamal If all these conditions, Which logic should I move in view model. For check network state, Network class is there. The only logic I use is showing no internet display and I did the same as done before in EvnetFragment. In event fragment I remove only one method of check internet state because it is already written in Network class and I call it directly. Now please suggest exactly what should I do?

iamareebjamal · 2018-09-29T18:10:52Z

The ViewModel should communicate with Activity/Fragment using LiveData only

liveHarshit · 2018-09-29T18:55:51Z

@iamareebjamal Updated.

iamareebjamal · 2018-09-30T08:11:10Z

app/src/main/java/org/fossasia/openevent/general/event/EventsFragment.kt

        }

-        showNoInternetScreen(isNetworkConnected())
+        showNoInternetScreen(eventsViewModel.isNetworkConnected())


The viewmodel should push the change to the observing fragment instead of you querying the viewmodel

The viewmodel should push the change to the observing fragment instead of you querying the viewmodel

Then which logic should I move in respective ViewModels? Here I can direct check network connection from network class.

Can I call it like showNoInternetScreen(searchViewModel.network.isNetworkConnected()) directly and make val network: Network insted of private val network: Network in view model?

Look at how we are communicating with LiveData in other places

liveHarshit · 2018-10-15T17:56:37Z

@iamareebjamal Updated.

simarsingh24 · 2018-11-27T13:09:15Z

@liveHarshit please resolve conflicts and update the PR. It's not nice to keep PR lying for month

liveHarshit · 2018-11-27T13:20:02Z

I will update it. Is the PR ready for merge at this solution or changes require?

iamareebjamal · 2018-11-27T15:43:08Z

As far as I can see, the previous requested change is not resolved

Fixes fossasia#283

liveHarshit · 2018-12-07T10:25:09Z

@iamareebjamal @simarsingh24 Everything is updated as suggested. Please review.

liveHarshit changed the title ~~feat: Show no internet error in search activity (#283)~~ feat: Show no internet error in search activity Sep 27, 2018

dreadpool2 reviewed Sep 27, 2018

View reviewed changes

app/src/main/java/org/fossasia/openevent/general/search/SearchFragment.kt Outdated Show resolved Hide resolved

dreadpool2 reviewed Sep 27, 2018

View reviewed changes

liveHarshit force-pushed the issue283 branch from c1e271a to 8bd642e Compare September 27, 2018 20:33

simarsingh24 reviewed Sep 28, 2018

View reviewed changes

app/src/main/java/org/fossasia/openevent/general/event/EventsFragment.kt Show resolved Hide resolved

liveHarshit force-pushed the issue283 branch 2 times, most recently from 7a6cbdf to b10ebd4 Compare September 28, 2018 05:30

simarsingh24 reviewed Sep 28, 2018

View reviewed changes

liveHarshit force-pushed the issue283 branch from b10ebd4 to 6a6c54d Compare September 28, 2018 09:21

shikhart98 suggested changes Sep 29, 2018

View reviewed changes

app/src/main/res/values/strings.xml Show resolved Hide resolved

iamareebjamal requested changes Sep 29, 2018

View reviewed changes

liveHarshit force-pushed the issue283 branch from 6a6c54d to 36f9921 Compare September 29, 2018 08:29

iamareebjamal requested changes Sep 29, 2018

View reviewed changes

liveHarshit force-pushed the issue283 branch from 36f9921 to 3b7294b Compare September 29, 2018 18:54

simarsingh24 previously approved these changes Sep 30, 2018

View reviewed changes

iamareebjamal requested changes Sep 30, 2018

View reviewed changes

liveHarshit dismissed simarsingh24’s stale review via 0a124fe October 15, 2018 17:30

liveHarshit force-pushed the issue283 branch 2 times, most recently from 0a124fe to 6446c16 Compare October 15, 2018 17:55

fossasia deleted a comment Oct 16, 2018

iamareebjamal added the under inspection label Oct 16, 2018

feat: Show no internet error in search activity (fossasia#283)

7f57007

Fixes fossasia#283

liveHarshit force-pushed the issue283 branch from 6446c16 to 7f57007 Compare December 7, 2018 10:24

iamareebjamal approved these changes Dec 7, 2018

View reviewed changes

iamareebjamal merged commit 179dc99 into fossasia:development Dec 7, 2018

liveHarshit deleted the issue283 branch January 6, 2019 16:55

feat: Show no internet error in search activity #592

feat: Show no internet error in search activity #592

Uh oh!

Conversation

liveHarshit commented Sep 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

liveHarshit commented Sep 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

shikhart98 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

iamareebjamal left a comment

Choose a reason for hiding this comment

Uh oh!

liveHarshit commented Sep 29, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

liveHarshit commented Sep 29, 2018

Uh oh!

iamareebjamal commented Sep 29, 2018

Uh oh!

liveHarshit commented Sep 29, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

liveHarshit Sep 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

liveHarshit commented Oct 15, 2018

Uh oh!

simarsingh24 commented Nov 27, 2018

Uh oh!

liveHarshit commented Nov 27, 2018

Uh oh!

iamareebjamal commented Nov 27, 2018

Uh oh!

liveHarshit commented Dec 7, 2018

Uh oh!

liveHarshit commented Sep 27, 2018 •

edited

Loading

liveHarshit commented Sep 27, 2018 •

edited

Loading

liveHarshit Sep 30, 2018 •

edited

Loading