-
Notifications
You must be signed in to change notification settings - Fork 686
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
feat: Third Party Beneficiaries Integration #312
Conversation
@Override | ||
public void detachView() { | ||
super.detachView(); | ||
subscription.unsubscribe(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dilpreet96 use subscription.clear()
instead
@Override | ||
public void detachView() { | ||
super.detachView(); | ||
subscription.unsubscribe(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dilpreet96 use subscription.clear()
instead
@Override | ||
public void detachView() { | ||
super.detachView(); | ||
subscription.unsubscribe(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dilpreet96 use subscription.clear()
instead
private List<Beneficiary> beneficiaryList; | ||
|
||
@Inject | ||
public BeneficiaryListAdapter(@ActivityContext Context context) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job for using @ActivityContext Context context
public class ViewHolder extends RecyclerView.ViewHolder { | ||
|
||
@BindView(R.id.tv_beneficiary_name) | ||
TextView tvName; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Give one line space between this and below variable
public class BeneficiaryDetailFragment extends BaseFragment implements BeneficiaryDetailView { | ||
|
||
@BindView(R.id.tv_beneficiary_name) | ||
TextView tvName; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leave one line gap
OnItemClickListener, SwipeRefreshLayout.OnRefreshListener, BeneficiariesView { | ||
|
||
@BindView(R.id.rv_beneficiaries) | ||
RecyclerView rvBeneficiaries; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leave one line gap
|
||
void showUserInterface(); | ||
|
||
public void showBeneficiaryTemplate(BeneficiaryTemplate beneficiaryTemplate); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
interface method by default are public, I don't think So you need to mention.
|
||
public interface BeneficiaryDetailView extends MVPView { | ||
|
||
public void showUserInterface(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
interface method by default are public, I don't think So you need to mention.
android:layout_width="match_parent" | ||
android:layout_height="match_parent" | ||
android:layout_margin="@dimen/default_margin"> | ||
<ScrollView |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dilpreet96 please use NestedScrollView
@therajanmaurya changes made |
Demo:
Please make sure these boxes are checked before submitting your pull request - thanks!
Apply the
MifosStyle.xml
style template to your code in Android Studio.Run the unit tests with
./gradlew check
to make sure you didn't break anythingIf you have multiple commits please combine them into one commit by squashing them.