-
Notifications
You must be signed in to change notification settings - Fork 16
#172. Redesign records list. #182
Conversation
yev-kanivets
left a comment
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 in general 👍Can be improved in few places, check comments :)
| companion object { | ||
|
|
||
| private const val KEY_ACCOUNT = "key_account" | ||
| private const val EMPTY_DATE = "empty_date" |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
|
||
| companion object { | ||
| private const val REQUEST_ACTION_RECORD = 6 | ||
| private const val EMPTY_DATE = "empty_date" |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| return countHeadersItems | ||
| } | ||
|
|
||
| private fun updateRecordAdapterDataList() { |
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.
Duplicated code.
| viewHolder.tvTitle.text = record.title | ||
| viewHolder.tvCategory.text = record.category?.name | ||
| viewHolder.tvCurrency.text = record.currency | ||
| if (rvViewHolder is RecordViewHolder) { |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
|
||
| @SuppressLint("SimpleDateFormat") | ||
| private static final SimpleDateFormat dateFormat = new SimpleDateFormat("yyyy-MM-dd"); | ||
| private static final SimpleDateFormat dateInNumberFormat = new SimpleDateFormat("yyyy-MM-dd"); |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| <item name="android:textColor">@color/text_black</item> | ||
| </style> | ||
|
|
||
| <style name="Text_Heading"> |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
|
||
| override fun initViews(view: View) { | ||
| recyclerView.adapter = RecordAdapter(requireContext(), getRecords(), false, null) | ||
| recyclerView.adapter = RecordAdapter(requireContext(), getRecordAdapterDataList(), false, null) |
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.
getRecordItems will be more appropriate, I believe :)
| fillDefaultAccount() | ||
| } | ||
|
|
||
| private fun getCountHeadersItems(position: Int): Int { |
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.
Smart, but I would take one more look on the places where we use getCountHeadersItems. Probably we can get a record without this magic :)
| private var summaryPresenter: ShortSummaryPresenter | ||
| private var isHeaderViewNeeded: Boolean = false | ||
| private var headerViewHolder: HeaderViewHolder | ||
| private var isSummaryViewNeeded: Boolean = false |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| } else { | ||
| ViewHolder(LayoutInflater.from(context).inflate(R.layout.view_record, parent, false), itemClickListener) | ||
| when (viewType) { | ||
| TYPE_RECORD -> RecordViewHolder(LayoutInflater.from(context).inflate(R.layout.view_record, parent, false), itemClickListener) |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@bogdan-evtushenko code is OK 🎉I'll do a functional review now. |
yev-kanivets
left a comment
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.
@bogdan-evtushenko functionally looks perfect 👍
No description provided.