Skip to content
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

EventHook - Question/Example #418

Closed
MFlisar opened this issue May 19, 2017 · 12 comments
Closed

EventHook - Question/Example #418

MFlisar opened this issue May 19, 2017 · 12 comments
Assignees
Labels

Comments

@MFlisar
Copy link
Contributor

MFlisar commented May 19, 2017

I've created following event hook:

 public static class SwitchEvent implements EventHook<IItem> {
    @Override
    public View onBind(@NonNull RecyclerView.ViewHolder viewHolder) {
        if (viewHolder instanceof SettingsNumberItem.ViewHolder) {
            return ((ViewHolder) viewHolder).binding.swEnable;
        }
        return null;
    }

    @Nullable
    @Override
    public List<? extends View> onBindMany(@NonNull RecyclerView.ViewHolder viewHolder) {
        return null;
    }

    public void onCheckedChanged(View v, int position, FastAdapter<SettingsNumberItem> fastAdapter, SettingsNumberItem item) {
        fastAdapter.notifyAdapterItemChanged(position);
    }
}

And following listener in my item:

 private CompoundButton.OnCheckedChangeListener mOnCheckedChangeListener = new CompoundButton.OnCheckedChangeListener() {
    @Override
    public void onCheckedChanged(CompoundButton compoundButton, boolean b) {
        // inform the event hook about the event
        // if (valueChanged) getSwitchEventHook().onCheckedChanged(...)
    }
};

I've following questions:

  1. I want that every item uses this event hook. So I just return the Switch view of every item on my onBind function, correct?
  2. Who calls the onCheckedChanged function? My mOnCheckedChangeListener should inform the event hook, but how??
@MFlisar MFlisar changed the title EventHook - Example EventHook - Question/Example May 19, 2017
@mikepenz
Copy link
Owner

mikepenz commented May 20, 2017

@MFlisar in your case you want to go with the CustomEventHook. The core only comes with a few pre-provided event hooks to keep it slim. So Click, LongClick, Touch.

If you want to have a custom event being bound properly to the Item's View. you should use the CustomEventHook and implement the onEvent method.

In onEvent you get the RecyclerView and set the listener to that view. That's only done once during ViewHolder creation. So the FastAdapter takes care of this.

If you need access to the item within the event you define you'll then need to check what is already done automatically for the Click EventHook for example:

//we get the adapterPosition from the viewHolder
int pos = fastAdapter.getHolderAdapterPosition(viewHolder);
//make sure the click was done on a valid item
if (pos != RecyclerView.NO_POSITION) {
    fastAdapter.getItem(pos); //this will give you the proper item at the current position
}

@mikepenz mikepenz self-assigned this May 20, 2017
@MFlisar
Copy link
Contributor Author

MFlisar commented May 20, 2017

I think the naming is not very good, but I got it working now. Additionally I wrote following base class for myself, I think this will help others to understand the event hooks a little easier.

Extended base class

public abstract class BaseCustomEventHook<T extends IItem> extends CustomEventHook<T> {

	// -------------------------
	// Interface
	// -------------------------

	/*
	 * return true, if the provided viewHolder is interested in this event hook
	 */
	public abstract boolean shouldEventHookBeUsed(RecyclerView.ViewHolder viewHolder);

	/*
	 * return the view of the viewHolder the event hook corresponds to
	 */
	public abstract View getEventHookView(@NonNull RecyclerView.ViewHolder viewHolder);

	/*
	 * bind the desired event hook listener to the view
	 * use it like following:
	 * view.setOnClickListener(view -> { onHandleCustomEvent(...); });
	 */
	public abstract void bindListener(View view, RecyclerView.ViewHolder viewHolder, FastAdapter<T> fastAdapter);

	public abstract void onEvent(View view, int pos, FastAdapter<T> fastAdapter, T item, RecyclerView.ViewHolder viewHolder, Object... params);

	// -------------------------
	// Helper function
	// -------------------------

	/*
	 * call this from within {@link bindListener} to tell the hook that the desired event just happened
	 */
	protected final void onEventOccurred(View view, RecyclerView.ViewHolder viewHolder, FastAdapter<T> fastAdapter, Object... params)
	{
		//we get the adapterPosition from the viewHolder
		int pos = fastAdapter.getHolderAdapterPosition(viewHolder);
		//make sure the click was done on a valid item
		if (pos != RecyclerView.NO_POSITION) {
			//we update our item with the changed property
			onEvent(view, pos, fastAdapter, fastAdapter.getItem(pos), viewHolder, params);
		}
	}

	// -------------------------
	// Default implementation
	// -------------------------

	@Override
	public final void onEvent(View view, RecyclerView.ViewHolder viewHolder, FastAdapter<T> fastAdapter) {
		bindListener(view, viewHolder, fastAdapter);
	}

	@Override
	public final View onBind(@NonNull RecyclerView.ViewHolder viewHolder) {
		if (shouldEventHookBeUsed(viewHolder))
			return getEventHookView(viewHolder);
		return null;
	}

	@Nullable
	@Override
	public List<View> onBindMany(@NonNull RecyclerView.ViewHolder viewHolder) {
		// TODO...
		return null;
	}
}

Usage

public static class EnableSettingsSwitchEvent extends BaseCustomEventHook<IItem>
{
    @Override
    public boolean shouldEventBeUsed(RecyclerView.ViewHolder viewHolder)
    {
        // Every ViewHolder should use this
        return true;
    }

    @Override
    public View getEventHookView(@NonNull RecyclerView.ViewHolder viewHolder)
    {
	    // return the view of the viewHolder the event is based on
        return ((ViewHolder) viewHolder).binding.swEnable;
    }

    @Override
    public void bindListener(View view, RecyclerView.ViewHolder viewHolder, FastAdapter<IItem> fastAdapter)
    {
        // forward custom event to {@link BaseCustomEventHook#onEventOccurred}
        ((SwitchCompatFixed)view).setOnCheckedChangeListener((v, b) -> onEventOccurred(v, viewHolder, fastAdapter, b));
    }

    @Override
    public void onEvent(View view, int pos, FastAdapter<IItem> fastAdapter, IItem item, RecyclerView.ViewHolder viewHolder, Object... params)
    {
        // handle the event
        ((SettingsItem)item).onUseCustomChanged((ViewHolder) viewHolder);
    }
}

I don't know what the onBindMany is meant for though. But I think the above base class or some variant of it would event fit into the library

@FabianTerhorst
Copy link
Contributor

This is just increasing the complexity of a simple 3 methods class. It should be enough to check the viewholder instance inside the onBind/onBindMany instead of creating a new method just for doing this.

@FabianTerhorst
Copy link
Contributor

You would also increase the performance when you just avoid the initialization of the anonymous inner class here and use the EventHook object as an replacement.

@MFlisar
Copy link
Contributor Author

MFlisar commented May 20, 2017

Was just an suggestion. For me the onBind does not tell me what it is expecting me to do. I miss the following informations:

  • I need to decide which item is interested in the event hook
  • I need to return a view that is the responsible view of the event
  • I need to set the listener for this view
  • I need to handle the event

With that in mind and with the custom event only providing a onBind method I was list. I had to check the code and find out what to do. And my extension is simplifying this, not from a method count view but from a new user view...

Still just a suggestion nothing more...

@FabianTerhorst
Copy link
Contributor

When you have suggestions about the method names or any other breaking stuff that would improve the usability of the eventhooks you could write them here down and maybe we could add them to #294.

@MFlisar
Copy link
Contributor Author

MFlisar commented May 21, 2017

You're right, a little documentation and you don't need that much methods. But I would suggest following:

  1. rename onEvent to onBindListener because with event I link the event of the hook in the sense of "hey, the hook event occurred, react to it`

  2. I would like to have a method that does following for me, because I think this is a default behaviour (every default hook does use it as well)

     int pos = fastAdapter.getHolderAdapterPosition(viewHolder);
     if (pos != RecyclerView.NO_POSITION) {
         onHandleEvent(view, pos, fastAdapter, fastAdapter.getItem(pos), viewHolder);
     }
    
  3. rename the onBind methods to something like getEventHookView or similar to more clearly explain what the method is expecting from me

Suggestion

public abstract class CustomEventHook<Item extends IItem> implements EventHook<Item> {

	/*
	 * the hooks event handler, will be called if the corresponding listener has fired an event for the corresponding item
	*/
	public abstract void onEvent(View view, int pos, FastAdapter<Item> fastAdapter, IItem item, RecyclerView.ViewHolder viewHolder);
	
	/*
	 * bind the event hook view to a listener
	*/
	public abstract void onBindListener(View view, RecyclerView.ViewHolder viewHolder, FastAdapter<Item> fastAdapter);

	/*
	 * return the view for this hook that the listener should be bound to
	 *
	 * @return null, if the provided ViewHolder should not be bound to the event hook, the view responsible for the event otherwise 
	*/
	@Nullable
	@Override
	public View getEventHookView(@NonNull RecyclerView.ViewHolder viewHolder) {
		return null;
	}

	/*
	 * return the views for this hook that the listener should be bound to
	 *
	 * @return null, if the provided ViewHolder should not be bound to the event hook, the view responsible for the event otherwise 
	*/
	@Nullable
	@Override
	public List<View> getEventHookManyView(@NonNull RecyclerView.ViewHolder viewHolder) {
		return null;
	}
	
	/*
	 * default implementation
	 */
	protected void onEventOccurred(View view, RecyclerView.ViewHolder viewHolder, FastAdapter<T> fastAdapter)
	{
		//we get the adapterPosition from the viewHolder
		int pos = fastAdapter.getHolderAdapterPosition(viewHolder);
		//make sure the click was done on a valid item
		if (pos != RecyclerView.NO_POSITION) {
			//we update our item with the changed property
			onEvent(view, pos, fastAdapter, fastAdapter.getItem(pos), viewHolder);
		}
	}
}

Maybe the onEventOccurred could also be placed in a utility class...

@FabianTerhorst
Copy link
Contributor

In most cases you don't need the position of the viewholder so you would have to add this manually when needed. You could add a pull request that just renames the methods with an documentation. Everything else is not needed for the default case.

@MFlisar
Copy link
Contributor Author

MFlisar commented May 22, 2017

You mean getting the item is not necessary, don't you? Is there a case where it's save not to check the position against RecyclerView.NO_POSITION?

@FabianTerhorst
Copy link
Contributor

Its not necessary to do this in a specific method because you need to provide an own listener implementation anyway.

@mikepenz
Copy link
Owner

I guess I can close this, as the documentation is added with this PR: https://github.com/mikepenz/FastAdapter/pull/425/files

@mikepenz
Copy link
Owner

I guess you should always check for the NO_POSITION as soon as you access the getAdapterPosition, as it might return it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants