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

Order groups by #58

Closed
xJon opened this issue Sep 15, 2020 · 20 comments
Closed

Order groups by #58

xJon opened this issue Sep 15, 2020 · 20 comments

Comments

@xJon
Copy link

xJon commented Sep 15, 2020

Hi, first of all thank you for creating a great & useful package.
I wanted to ask - is there a way to order the groups by a custom (and not alphabetical) order?
Thank you!

@xJon
Copy link
Author

xJon commented Sep 15, 2020

A minute later I find the sort option - setting it to false, I am able to order the groups by presorting my list.
Thank you nonetheless.

@xJon xJon closed this as completed Sep 15, 2020
@xJon
Copy link
Author

xJon commented Sep 15, 2020

Actually, it's not the perfect solution. I'd like to know what you think about the feature!

@xJon xJon reopened this Sep 15, 2020
@Dimibe
Copy link
Owner

Dimibe commented Sep 15, 2020

Hi @xJon,

currently there is no custom sorting option available.

The groups and the elements inside each group are sorted according their comparable implementation. So for example when using Dates the groups will be sorted by date and not alphabetically. The groupHeaderBuilder could then display the weekday or something.
For custom sorting you could override the compareTo method of your groupBy value.

What is your groupBy value? Will this solution work for you?

A solution which was discussed before is to add an sortBy option, which returns for two group a Int value defining the order. So basically the like comparable Interface with the only difference that with this solution a separate sorting for the list can be done.
In my point of view it's better to override the compareTo method on the groupBy value.

Let me know what you think.

@xJon
Copy link
Author

xJon commented Sep 15, 2020

Hi, thank you for the quick and detailed response.
I am grouping my items according to their pre-assigned category names, so currently they are only sorted alphabetically.
My plan was to sort the categories according to the DateTime assigned to each item. I'm not sure how I'd do that with what you were suggesting?

@Dimibe
Copy link
Owner

Dimibe commented Sep 17, 2020

@xJon I mean't that you can do something like this:

groupBy: (element) => DateFormat('yyyy-MM-dd').format(element.date),
groupHeaderBuilder: (Element element) => Text(
   '${DateFormat.yMMMd().format(element.date)}',
   textAlign: TextAlign.center,
    style: TextStyle(fontSize: 20, fontWeight: FontWeight.bold),
),

The List is grouped by the actual date (without time) and then in groupHeaderBuilder you format the Date in another way. So this example will lead to this outcome:
image

As you can see the months are displayed and in chronological order and not in alphabetical order.
If you like to you can also display week days or anything else. The point is that groupBy defines the order and groupHeaderBuilder the visible widget.

@xJon
Copy link
Author

xJon commented Sep 17, 2020

I see. That is a useful option but it isn't applicable in my case, as I am trying to use groupBy in one way, and then order the groups in another way. Thank you nonetheless for the support!

@Dimibe
Copy link
Owner

Dimibe commented Sep 17, 2020

@xJon No problem, what exactly want you to do?
On way of archiving your described use case may be this:

GroupedList<Element, Group> ( 
  groupBy: (element) => element.groupByValue
  [...]
),

[...]

class Group implements Comparable {
  var sortValue;

  [...]

  @override
  int compareTo(other) {
    return sortValue.compareTo(other.sortValue);
  }
}

As you can see the list is grouped by a field of the element. This field's class implements the Comparable interface. There a custom comparison is implemented.
Does this solves your problem?

@deakjahn
Copy link

deakjahn commented Sep 18, 2020

I came here because of the same issue. The idea would be OK but it seems the execution is not yet perfect. :-) Even if I have compareTo() return zero for groups that I consider the same, they will appear separately. I have a hunch that you don't use compareTo() in every single place needed, so two groups that have the same contents but are different actual objects get treated separately. I'm still chasing it...

@deakjahn
Copy link

deakjahn commented Sep 18, 2020

Indeed. If I create a little helper:

bool _equal(E e1, E e2) {
  var compareResult;
  if (e1 is Comparable) {
    compareResult = (e1 as Comparable).compareTo(e2 as Comparable);
  }
  if ((compareResult == null || compareResult == 0) && e1 is Comparable) {
    compareResult = (e1).compareTo(e2);
  }
  return compareResult == 0;
}

And use it in both places where you directly compare curr and prev (in _scrollListener() and build(), it starts to work just fine.

@Dimibe
Copy link
Owner

Dimibe commented Sep 18, 2020

@deakjahn Yes, I use compareTo only for sorting. For grouping the equality operator is used.
So intended solution would be to also override the == operator (see dart doc) in your class.

@Dimibe
Copy link
Owner

Dimibe commented Sep 18, 2020

Since the procedure seems a little confusing I thought of a solution and is my proposal:

A new option sortGroupBy will be added which takes a function: int Function(E group1, E group2)

GroupedList<Element, Group> ( 
  groupBy: (element) => element.groupByValue
  sortGroupBy: (group1, group2) => [..] //  needs to return an int in range [-1, 1] (same as `compareTo`).
  [...]
),

@deakjahn, @xJon What do you think, would that solve your problem?

@deakjahn
Copy link

deakjahn commented Sep 18, 2020

Fine. I would actually suggest this over the == approach because forcing to override might not be a good choice. Who knows, people might not want to create a simple class just for the group but they might try to reuse some larger class they already have and overriding equality might upset the applecart for them. Better leave it alone and ask for the comparison specifically.

@Dimibe
Copy link
Owner

Dimibe commented Sep 18, 2020

To be clear here, the new option will only be used for sorting.
When returning zero the only effect will be that the order of the two compared groups is not defined.

So with that new option you must not implement and override the
Comparable interface. This might help in use cases where the group value is not a dedicated class but just some data.

If you want two groups to be treated as the same the only way is and will be that they are equal in terms of the == Operator. In my eyes there is no need to change that.

@deakjahn
Copy link

Yes, confirmed, it works that way. If you just mention this in the source comments around groupBy, and with the new addition, it should cover all cases.

@Dimibe
Copy link
Owner

Dimibe commented Sep 19, 2020

Closed with #59

@Dimibe Dimibe closed this as completed Sep 19, 2020
@xJon
Copy link
Author

xJon commented Sep 21, 2020

Hi @Dimibe, thank you for the new feature!
It is useful but I'm pretty sure that in my case it's non-applicable, as the groupComparator() function provides the same values given to the groupBy() function.
I'm using groupBy() to group my items according to their pre-assigned category names, but then my intention is to sort the groups according to the groups' latest-added item timestamp (which I've pre-sorted to a DateTime value for each group's first item). Could groupComparator() let me compare according to the group's contents, or first item?
Hopefully I've been clear enough with what I mean.

@Dimibe
Copy link
Owner

Dimibe commented Sep 21, 2020

@xJon No that's not possible because it is not known what item is "first" during sorting. This information is obviously only accessible after the list is sorted once. So the list would have to be sorted twice.
I think this is a very special use case so I won't implement a solution in this package.

What you could do is to store the "timestamps" of each group manually (e.g. in a map) and then access them in the groupComparator function.

@xJon
Copy link
Author

xJon commented Sep 21, 2020

@Dimibe Looks like a good solution, thank you.

@xJon
Copy link
Author

xJon commented Sep 22, 2020

@Dimibe I got it to work exactly as wanted with your advice!

@TJMusiitwa
Copy link

TJMusiitwa commented Sep 22, 2021

@xJon and @Dimibe Sorry to bring you back to this issue but I am in a very similar pickle to this. I have the items grouped and all that I am fetching from firestore.
The issue is that when the grouping occurs, it creates very different groups yet I would like something similar to what is in #117.
I guess I am seeking help in the best course of action to do a group comparator.

Or maybe @xJon you would be so kind as to offer some detail into the solution you used to got this working

The image below can give you a clue of what I am facing.

My groupBy argument:

groupBy: (entry) =>
              DateTime.fromMillisecondsSinceEpoch(entry['creationTime'] as int)
                  .toString()
                  .substring(0, 10),

With the basic logic being

final now = DateTime.now();
                  final today = now.difference(segmentDate).inDays == 0;
                  final yesterday = now.difference(segmentDate).inDays == 1;
                  final sevenDays = now.difference(segmentDate).inDays <= 7;
                  final thirtyDaysAgo =
                      now.difference(segmentDate).inDays <= 30;

Inside my group separator builder, I have several conditionals similar to this:

if (today) {
                    return Container(
                        height: 30,
                        color: LvndrColors.background,
                        child: const Padding(
                          padding: EdgeInsets.only(left: 18),
                          child: LvndrText.captionLarge('Today'),
                        ));
                  } else if (yesterday) {
                    return Container(
                        height: 30,
                        color: LvndrColors.background,
                        child: const Padding(
                          padding: EdgeInsets.only(left: 18),
                          child: LvndrText.captionLarge('Yesterday'),
                        ));
                  }

Simulator Screen Shot - iPhone 13 Pro - 2021-11-04 at 16 29 10

I hope you can guide me on the missing step. Thanks

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

No branches or pull requests

4 participants