-
Notifications
You must be signed in to change notification settings - Fork 604
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: fetch & show notes for client and group, fixes #543 #557
Conversation
a68edb0
to
12f2801
Compare
@@ -54,6 +55,7 @@ | |||
private static StaffService staffApi; | |||
private static SurveyService surveyApi; | |||
private static RunReportsService runreportsService; | |||
private static NoteItemService noteItemService; |
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.
Use Proper Naming convention. This should be like NoteService
* Created by rahul on 4/3/17. | ||
*/ | ||
|
||
public class NoteItemAdapter extends RecyclerView.Adapter<NoteItemAdapter.ViewHolder> { |
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.
Class name should be NoteAdapter
/** | ||
* Created by rahul on 4/3/17. | ||
*/ | ||
public class NoteItemFragment extends MifosBaseFragment implements NoteItemMvpView, |
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.
This class name should be NoteFragment
RecyclerView rvNote; | ||
|
||
@BindView(R.id.progressbar_note) | ||
ProgressBar pbNote; |
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.
No need to add the ProgressBar in UI, Use MifosBaseFragment Dynamic ProgressBar.
You can access using the showMifosProgressBar();
and hide by hideMifosProgressBar();
ImageView ivError; | ||
|
||
@BindView(R.id.ll_error) | ||
LinearLayout ll_error; |
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.
Failed name should be llError
@Override | ||
public void onDestroyView() { | ||
super.onDestroyView(); | ||
noteItemPresenter.detachView(); |
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.
When you will add MifosBaseFragment Progressbar then do not forget to add hideMifosProgressBar();
here.
*/ | ||
public class NoteItem extends MifosBaseModel implements Parcelable { | ||
|
||
private int id; |
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.
Annotate fields with @SerializedName("")
and remove private identifier.
|
||
void showUserInterface(); | ||
|
||
void showNoteItem(List<NoteItem> noteItem); |
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.
Method name should be showNote(List<NoteItem> noteItem)
@rahulbabbar1 Please make the requested changes. |
@therajanmaurya i will fix them soon. |
12f2801
to
d3e10f4
Compare
d3e10f4
to
9f07605
Compare
@therajanmaurya I have fixed the requested changes. Please check. |
Good Work 👍 |
Please make sure these boxes are checked before submitting your pull request - thanks!
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.
Added option to show Notes of the Client and group in the toolbar menu for ClientDetailsFragment and GroupDetailsFragment.