Skip to content
This repository has been archived by the owner on Mar 4, 2020. It is now read-only.

Adapter method updates #41

Merged
merged 3 commits into from
Feb 27, 2017
Merged

Adapter method updates #41

merged 3 commits into from
Feb 27, 2017

Conversation

zplesac
Copy link
Member

@zplesac zplesac commented Feb 24, 2017

Removed deprecated methods in order to prepare Mjolnir for 2.0.0 release.

Also, I've renamed addFooter/addHeader methods to setFooter/setHeader, as they are more precise - if we use "add" keyword, it immediately signals that we are supporting multiple headers/footers, which is still not the case.

@reisub
Copy link

reisub commented Feb 24, 2017

Looks good, but I don't think the shouldReplace argument brings any value here. The set semantics kind of imply that replacement will be done.

@zplesac
Copy link
Member Author

zplesac commented Feb 24, 2017

@reisub this is opened for discussion. In that case, we should also refactor setFooter and setHeader methods to return type void.

@mariciv @ikocijan what's your opinion about this?

@mariciv
Copy link
Member

mariciv commented Feb 24, 2017

I agree with @reisub shouldReplace is unnecessary. Return type can be void and you need to update javadoc.

Do we have a removeHeader/Footer methods? If not I think we should have it. There could be a need to remove it after inital set, do you agree?

@reisub
Copy link

reisub commented Feb 24, 2017 via email

@zplesac
Copy link
Member Author

zplesac commented Feb 24, 2017

@reisub @mariciv

Agreed - I'll take a closer look into ListView API for adding/removing headers and will try to mimic it's behaviour.

headerView = LayoutInflater.from(getContext()).inflate(headerViewId, null, false);
setDefaultLayoutParams(headerView);

if (hadHeaderBefore) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could call setHeader(View) method to avoid duplicate logic.

boolean hadFooterBefore = hasFooter();

int position = getCollectionCount() + (hasHeader() ? 1 : 0);
footerView = LayoutInflater.from(getContext()).inflate(footerViewId, null, false);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same as for the header. You can call setFooter(View) to avoid duplicate logic.

@zplesac
Copy link
Member Author

zplesac commented Feb 27, 2017

@reisub all comments have been resolved.

Copy link

@reisub reisub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks very clean now!

@zplesac zplesac merged commit 32c2092 into master Feb 27, 2017
@zplesac zplesac deleted the feature/update-methods branch February 27, 2017 16:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants