Error View as a View in MvpLce implementations?

[BillBosiolis]

Currently MvpLceFragment (and the other similar classes) expects to find a TextView with an id of errorView.

The abstract class contains most of the required logic and the developer only needs to implement the getErrorMessage() to provide a suitable message to the user in case of an error.

But what if you change its type to be a View? Of course this will break the API but it will let the developer use a custom view or view group. The view can be anything (fancy group with background images, TextView etc).

In that case the developer should implement a new abstract method, called let's say, prepareErrorView() which will be used by the modified showError() of the ancestor which will only make the 'errorView' visible.

What do you think?

[sockeqwe]

We already had this discussion earlier in #42

Well, I'm open to discuss and change that but only if enough user of mosby are requesting this.

Alternatively: You don't have to use MvpLceFragment etc. if you don't need them. You can always write your own.

The idea behind LCE is to have a quick out-of-the box solution by following some conventions. One of them is that ErrorView has to be a TextView because it covers most use cases. If you want something more customizeable then you should consider to write your own class that extends from MvpFragment(without LCE).

[BillBosiolis]

Hey @sockeqwe,

I only searched the open issues so I didn't find #42.

Well I denitelety agree that TextView covers most cases.

So for now, I will either use compound drawables (I can always override the showError() and set a case specific drawable) or implement my 'own' version of MvpLceFragment which will actually use the exact code as the original one but will work with a 'generic' error View.

Thanks for your quick response and your great library (yes, it's not a framework). It has saved me a looot of time :-)

I think you can close this issue.

[sockeqwe]

No worries, it's hard to find similar issues in githubs issue tracker.

So I will close this for now, but as already said, if some more users are demanding it, I'm open to reopen this issue and to discuss this.

[matoelorriaga]

Hello @sockeqwe, I'm using your library in my application, and I override some methods in MvpLceFragment to support using a View instead a TextView.

Anyway, I'm thinking in implement my own version of MvpLceFragment to support that.

I'm in favour of adding it in Mosby, I can help you with that.

Regards, Matias

[sockeqwe]

Hi @matoelorriaga , well, it's unlikely that I will change the current LCE implementation because I think it will add to much boilerplate code when changing error view to a generalized View (not TextView), but I'm willing to listen to your proposal and then we can decide if we want to merge it into Mosby.

How would you deal with error string messages, retry click, toast in case of pull to refresh etc? What will be left to the user of the MvpLceFragment to implement?

[mrPjer]

I stumbled upon this by trying to make my error view richer by adding additional elements such as a title and an image.

The current implementation is quite limited and does not allow this, so I patched up MvpLceViewStateFragment(https://github.com/mrPjer/mosby/commit/b003274f8155db6f8257b5f62e1841e818cfb85a). The patch adds in the concept of a container for the error view which is just a View and can contain anything, as long as the standard errorView is present. If no container is specified, then the errorView is used as the container.

This means that there is no overhead for users who do not want to expand their error views, but it is also quite easy to do so for those who do.

After playing around with it, I realized that I can achieve the same thing by overriding the methods animateLoadingViewIn, animateContentViewIn and animateErrorViewIn, as well as attaching a click handler in onViewCreated in my own class. It's worth mentioning that doing it this way requires going through the same process for Activities extending MvpLceActivity.

On one hand, I'd like to see more flexibility in the default classes, but you are right when saying that the default behaviour can be overridden for specific needs.

Here is the final class of such a Fragment:

package com.eug2016.android.widget

import android.os.Bundle
import android.view.View
import com.eug2016.android.R
import com.hannesdorfmann.mosby.mvp.MvpPresenter
import com.hannesdorfmann.mosby.mvp.lce.LceAnimator
import com.hannesdorfmann.mosby.mvp.lce.MvpLceView
import com.hannesdorfmann.mosby.mvp.viewstate.lce.MvpLceViewStateFragment
import org.jetbrains.anko.onClick

abstract class EUGMvpLceViewStateFragment<CV : View, M, V : MvpLceView<M>, P : MvpPresenter<V>> : MvpLceViewStateFragment<CV, M, V, P>() {

    private lateinit var errorViewContainer: View

    override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
        super.onViewCreated(view, savedInstanceState)
        errorViewContainer = view.findViewById(R.id.errorViewContainer)
        errorViewContainer.onClick { onErrorViewClicked() }
    }

    override fun animateLoadingViewIn() {
        LceAnimator.showLoading(loadingView, contentView, errorViewContainer)
    }

    override fun animateContentViewIn() {
        LceAnimator.showContent(loadingView, contentView, errorViewContainer)
    }

    override fun animateErrorViewIn() {
        LceAnimator.showErrorView(loadingView, contentView, errorViewContainer)
    }

}

Here is an image of an errorView which this lets me build:

And the relevant error_view.xml:

<?xml version="1.0" encoding="utf-8"?>
<merge xmlns:android="http://schemas.android.com/apk/res/android"
       android:layout_width="match_parent"
       android:layout_height="match_parent">

    <FrameLayout
        android:id="@+id/errorViewContainer"
        android:layout_width="match_parent"
        android:layout_height="match_parent">

        <LinearLayout
            android:layout_width="match_parent"
            android:layout_height="wrap_content"
            android:layout_centerInParent="true"
            android:layout_gravity="center"
            android:orientation="vertical"
            android:paddingBottom="@dimen/errorTextContainerMarginBotton"
            android:paddingLeft="@dimen/errorTextContainerMarginLeft"
            android:paddingRight="@dimen/errorTextContainerMarginRight"
            android:paddingTop="@dimen/errorTextContainerMarginTop">

            <TextView
                android:layout_width="match_parent"
                android:layout_height="wrap_content"
                android:layout_gravity="center"
                android:gravity="center"
                android:text="@string/loading_error_title"
                android:textColor="@android:color/black"
                android:textSize="32sp"
                />

            <TextView
                android:layout_width="match_parent"
                android:layout_height="wrap_content"
                android:layout_marginBottom="16dp"
                android:layout_marginTop="16dp"
                android:gravity="center"
                android:text="@string/loading_error_message"
                />

            <TextView
                android:id="@+id/errorView"
                android:layout_width="match_parent"
                android:layout_height="wrap_content"
                android:layout_gravity="center"
                android:drawablePadding="16dp"
                android:gravity="center"
                android:padding="16dp"
                android:textColor="#656565"
                android:textSize="12sp"
                />

        </LinearLayout>

        <ImageView
            android:id="@+id/sticker"
            android:layout_width="wrap_content"
            android:layout_height="wrap_content"
            android:layout_alignParentBottom="true"
            android:layout_alignParentRight="true"
            android:layout_gravity="bottom|right"
            android:layout_marginBottom="16dp"
            android:layout_marginRight="16dp"
            android:src="@drawable/hrki_connect_error"
            />

    </FrameLayout>

</merge>