-
-
Notifications
You must be signed in to change notification settings - Fork 458
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
Feature/multi selection support + selection handler refacoring #50
base: master
Are you sure you want to change the base?
Feature/multi selection support + selection handler refacoring #50
Conversation
- now selection state sit in each Cell (instead of the CellRecyclerView, so the recycler doesn't get confuse) - introduced a new ISelectableModel interface, that need to be implemented by Cell, RH and CH in order to get a selection state - introduced multiple selection
fixed multi-selection clear when change between cell , row or column selection introduced multiselection in tableview sample
6c8362a
to
543ccea
Compare
…nt UI issue with sorting
…plement in Cell, RowHeader or ColumnHeader
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.
fixed conflict with README, ready to review
@@ -49,8 +49,8 @@ protected boolean clickAction(RecyclerView view, MotionEvent e) { | |||
int row = holder.getAdapterPosition(); | |||
|
|||
// Control to ignore selection color | |||
if (!mTableView.isIgnoreSelectionColors()) { | |||
mSelectionHandler.setSelectedRowPosition(holder, row); | |||
if (!mTableView.isIgnoreSelectionColors() && mTableView.isSelectable()) { |
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.
Should we remove "isIgnoreSelectionColors()" and keep "isSelectable()", I've the feeling it's accomplishing the same purpose.
Hi @sonique6784 These days, I am pretty busy. So, I haven't reviewed your codes. That's why I didn't approve it yet. I just want to evaluate your approach by a cup of coffee at a more relaxed time. After the reviewing process, I'll give you feedback. Thank you very much for your collaboration. 👍 |
No worries, it's a big chunk of code, take your time. thanks |
fixed conflict, ready to merge |
merge back
Hi @evrencoskun |
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.
I've added some wording/code style comments. Feel free to take them into account if you wish.
I didn't really look at the logic itself.
You also have some conflicts to solve.
...c/main/java/com/evrencoskun/tableview/adapter/recyclerview/RowHeaderRecyclerViewAdapter.java
Outdated
Show resolved
Hide resolved
...c/main/java/com/evrencoskun/tableview/adapter/recyclerview/RowHeaderRecyclerViewAdapter.java
Outdated
Show resolved
Hide resolved
/** | ||
* Created by cedricferry on 8/2/18. | ||
*/ | ||
|
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.
Remove empty line
|
||
public interface ISelectableModel { | ||
AbstractViewHolder.SelectionState getSelectionState(); | ||
void setSelectionState(AbstractViewHolder.SelectionState selectionState); |
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.
Add an empty line before the method
* Created by cedricferry on 8/2/18. | ||
*/ | ||
|
||
public interface ISelectableModel { |
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.
If you can, it'd be nice to have the method of this interface annotated with @NonNull
/@Nullable
, based on your needs.
Co-Authored-By: Gaëtan Muller <[email protected]>
Co-Authored-By: Gaëtan Muller <[email protected]>
Co-Authored-By: Gaëtan Muller <[email protected]>
Co-Authored-By: Gaëtan Muller <[email protected]>
suggestion from MGaetan89 Co-Authored-By: Gaëtan Muller <[email protected]>
thanks to MGaetan89 Co-Authored-By: Gaëtan Muller <[email protected]>
moved the vertical detection AFTER checking if it the same RV. else it could set the mCurrentRVTouched to null and lead to unsync scrolling.
…n. This problem has been fixed.
… to pom xml file Also bumped version
# Conflicts: # README.md # app/src/main/java/com/evrencoskun/tableviewsample/MainFragment.java # app/src/main/java/com/evrencoskun/tableviewsample/tableview/model/Cell.java # tableview/src/main/java/com/evrencoskun/tableview/ITableView.java # tableview/src/main/java/com/evrencoskun/tableview/adapter/recyclerview/CellRecyclerViewAdapter.java # tableview/src/main/java/com/evrencoskun/tableview/adapter/recyclerview/CellRowRecyclerViewAdapter.java # tableview/src/main/java/com/evrencoskun/tableview/adapter/recyclerview/ColumnHeaderRecyclerViewAdapter.java # tableview/src/main/java/com/evrencoskun/tableview/adapter/recyclerview/RowHeaderRecyclerViewAdapter.java # tableview/src/main/java/com/evrencoskun/tableview/handler/SelectionHandler.java # tableview/src/main/java/com/evrencoskun/tableview/listener/itemclick/CellRecyclerViewItemClickListener.java
# Conflicts: # README.md # app/src/main/java/com/evrencoskun/tableviewsample/MainFragment.java # app/src/main/java/com/evrencoskun/tableviewsample/tableview/model/Cell.java # tableview/src/main/java/com/evrencoskun/tableview/ITableView.java # tableview/src/main/java/com/evrencoskun/tableview/adapter/recyclerview/CellRecyclerViewAdapter.java # tableview/src/main/java/com/evrencoskun/tableview/adapter/recyclerview/CellRowRecyclerViewAdapter.java # tableview/src/main/java/com/evrencoskun/tableview/adapter/recyclerview/ColumnHeaderRecyclerViewAdapter.java # tableview/src/main/java/com/evrencoskun/tableview/adapter/recyclerview/RowHeaderRecyclerViewAdapter.java # tableview/src/main/java/com/evrencoskun/tableview/handler/SelectionHandler.java # tableview/src/main/java/com/evrencoskun/tableview/listener/itemclick/CellRecyclerViewItemClickListener.java
@MGaetan89 @evrencoskun , I've fixed the conflicts and rested, it looks fine now. |
@sonique6784 @evrencoskun |
Note: previous implementation was storing selection state into [Cell|RowHeader|ColumnHeader]ViewRecycler, which leaded to inconsistency in rendering (because RecyclerViews are re-used).