Skip to content
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

Add Categories #65

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

tobiasKaminsky
Copy link

This adds support for categories:
2020-05-17-080859

Currently missing is UI for changing/adding/resorting categories.
This can be done via editing plain text file, for now.

Signed-off-by: tobiasKaminsky [email protected]

Signed-off-by: tobiasKaminsky <[email protected]>
Copy link
Owner

@woefe woefe left a comment

Choose a reason for hiding this comment

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

I think it is problematic that the GUI is quite disconnected from the underlying datastructure. The List in the GUI is sectioned, but the text file is just a flat list. This introduces problems when reordering items. Moving them across sections would move them to other categories (if that worked). And the index/position gets messed up.

What do you think about adding a new field to each list item with a tiny font (and additionally color coding). And then adding new sorting functionality to sort by category? And then maybe extend the app settings to let the user define (multiple) orderings of categories?

@@ -44,4 +44,5 @@ dependencies {
implementation 'com.android.support:design:28.0.0'
implementation 'com.android.support:preference-v7:28.0.0'
implementation 'com.android.support:recyclerview-v7:28.0.0'
implementation 'com.afollestad:sectioned-recyclerview:0.5.0'
Copy link
Owner

Choose a reason for hiding this comment

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

I'm not too happy about this dependency, since it seems to be pretty much dead. The author even removed it from his GitHub profile

Copy link
Author

Choose a reason for hiding this comment

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

I know, this is totally outdated, but also used in NC Files App…
If you find a proper replace, I am more than happy to change it.
(also the other way around, if I find one for NC Files App).
But for now it "just works" (for >500k users…)

public void move(int oldIndex, int newIndex) {
public void move(ListItem from, ListItem to) {
int oldIndex = indexOf(from);
int newIndex = indexOf(to);
Copy link
Owner

Choose a reason for hiding this comment

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

This could be problematic. What if an item has been added twice? The GUI probably creates new objects for every item, though. What was the rationale for this change?

Copy link
Author

Choose a reason for hiding this comment

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

This addresses the problem you described above: GUI is disconnected from flat last.
Instead of relying on indexes, this is now really using the objects as stored in ArrayList.
Therefore it uses the real object and moves them in the List.
(upon write it then just goes through all ordered categories and writes the items to list.

ItemViewHolder sourceViewHolder = (ItemViewHolder) viewHolder;
ItemViewHolder targetViewHolder = (ItemViewHolder) target;

RecyclerListAdapter.this.move(sourceViewHolder.listItem, targetViewHolder.listItem);
Copy link
Owner

Choose a reason for hiding this comment

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

Not entirely sure where it was introduced, but when moving items, it only swaps with the neighbor item.

Copy link
Author

Choose a reason for hiding this comment

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

I will have a look 👍

@tobiasKaminsky
Copy link
Author

I think it is problematic that the GUI is quite disconnected from the underlying datastructure. The List in the GUI is sectioned, but the text file is just a flat list. This introduces problems when reordering items. Moving them across sections would move them to other categories (if that worked). And the index/position gets messed up.

I think one way to solve this is to read all items and store them in an array list (we could also use one List for each category).
Then GUI operates solely on those List(s).
Upon close it writes the data, sorted by categories and in the desired order (e.g A-Z or manually sorted).
Any operation on the items needs then to be done by object and not by indices.

What do you think about adding a new field to each list item with a tiny font (and additionally color coding). And then adding new sorting functionality to sort by category? And then maybe extend the app settings to let the user define (multiple) orderings of categories?

I think this just clutters the UI.
Of course I will enhance the UI of category headlines.

This is a screenshot from Out of Milk, which shows a clean and structured view of categories (imho)
Screenshot_20200518-215440_Out_of_Milk

@tobiasKaminsky
Copy link
Author

UPDATE:
So indeed sorting is broken.
I had a quick test and I guess we would need to use one List for each category.
At least this way it is working to sort correctly.

What do you think?
As this is quite a big change, I would like to ask before working further on it.

Signed-off-by: tobiasKaminsky <[email protected]>
@tobiasKaminsky
Copy link
Author

update: Sorting is now working again 🎉

Signed-off-by: tobiasKaminsky <[email protected]>
Signed-off-by: tobiasKaminsky <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants