Feature request: Allow turning off "no sort" state

[ykrasik]

Hi @caduandrade :)

I'd like to request the ability to turn off the table's "no sort" state. This would require the table to have a sort column from the moment it is initialized, and disallow having less than 1 sort column.
Currently, each header is clickable 3 times (in single column sort), with the 3rd click turning it off. I'd like to be able to disable that - this may only make sense for single column sort, not sure. It would mean:

• There must be 1 column with either a down or an up arrow always visible.
• Each header is clickable only 2 times and each click toggles the sort of that column. Not sure what the correct behavior with multi column sort would be.
• The only way, in single column sort, to deselect a column for sorting is to click on another column.

[caduandrade]

Hi! Nice use case! Make sense.

Maybe it could have 2 new attributes:

• alwaysSorted (bool). true is "no sort" state.
• defaultSort (List of ColumnSort). Where ColumnSort contains columnIndex and order.

If alwaysSorted is true and defaultSort is empty, it will automatically sort first ascending.

Maybe this will work for single or multiple sorting. In simple, it will use only the first item of the defaultSort list.

[caduandrade]

Maybe the defaultSort is not necessary or it conflicts with the attributes of the column itself that already define the order: order and priority.

I will take a look.

[caduandrade]

Sorry, I'm still working on my other packages as well.

Let's talk about this issue. I think I'll have to make the change regarding the column index as you mentioned in #68.

I'm trying to compare two different styles for sorting methods:

1. In the model. For the internal header Widget, it is convenient to use the method passing the Column. But for the developer, maybe it would be better to use a reference. Maybe the column id?
2. On the column. It could have setters that would notify the model. But to do multiple sorting would trigger multiple notifications. Thinking in multiple sorts, the first style makes more sense.

I thought of something like this in the model:

model.sort([DavidSort.ascending('name'), DaviSort.descending('age')]);

The String parameters are dynamic id

[ykrasik]

Of course, I wouldn't expect instant solutions :)

If I understand 2. correctly - it would be less convenient to use, as it would require developers to keep a reference to each DaviColumn as a field in the widget in order to set its sort value programatically.

Relying on the id looks more ergonomic, and it allows to extract that id into a const so the IDE can highlight which column that id is used in.

One thing about DaviSot.ascending/descending - it'd be nice to also have a DaviSort.of(id, direction) function, as I have my own SortDirection enum (and I imagine other devs who want to delegate the sorting logic outside of the table will have a similar setup), so it'd be a bit less code to only translate the SortDirection into DaviSortDirection, as opposed to using a different method instead. Not a big deal, though.

[caduandrade]

It's good, right?

model.sort([DaviSort('id1'), DaviSort('id2', DaviSortDirection.descending)]);

Now it remains to make the alwaysSorted attribute work.

By the way, I liked the suggestion of class names 😉

[ykrasik]

Looks great, happy to help ;)

[caduandrade]

Hi @ykrasik!

If you want, you can test the branch now. Just use the model's alwaysSorted attribute. Other changes are in the CHANGELOG.

Before releasing the version, I still have to create more tests and need to update the examples in the README.

In the future, the behavior to turn off sorting in multi-sorting could be configured. Currently, it is just clicking. Maybe it could be holding SHIFT or CTRL. But that can be another issue 😄 .

[ykrasik]

Hi @caduandrade!

Started playing with it. First of all, looks great! I was already able to achieve what I wanted. A few nits:

• Because the onSort callback passes a list of DaviColumn, but model.sort is called using DaviSort, I think it'd be a bit nicer if the columns also contained a DaviSort field (or exposed a getter that returns a DaviSort), as currently I need to manually build a DaviSort from the column. The reason for this is that I want to deal with a single entity for sorting - I have my own domain class that I use everywhere, and in interactions with Davi it is translated into a DaviSort. I think it would be cleaner if it worked the same way in the opposite flow of data - Davi.onSort notifies using DaviSort (or DaviColumn from which it is a bit easier to extract a DaviSort), my handler translates it into my domain class and passes it on.
• DaviSort is already basically a data class, maybe add == and hashCode to it? That would allow to do equality checks. And a toString for easier debugging.
• Would be nice if calling model.sort with the already existing sort would be detected and do nothing
• I think the directions are reversed - DaviColumn.direction == DaviSortDirection.ascending shows a header arrow pointing down.

Most of this is from #68 :)

[caduandrade]

Great ideas! I will do that.

About the arrow icon, this is funny because I understand that the ascending order is the arrow pointing down (starting at the top and growing down the screen) but now I realized that there are both representations: https://www.flaticon.com/free-icons/sort-ascending (what a mess). Google Material Icons doesn't have any icons for sorting, just generic arrows. Maybe it's better not to use arrows and switch to those bigger and smaller horizontal bars. Or "AZ" and "ZA" (Google Sheets). 🤔
It's already configurable via theme but it would be nice to have a default without causing confusion.

I'm still on tests and correcting some behavior. Example: alwaysSorted=true, but all columns with sortable=false, which means no columns are sorted.

[caduandrade]

The DaviColumn with a DaviSort is a good idea.

Now:

DaviColumn.sort (DaviDataComparator)
DaviColumn.sortPriority (int)
DaviColumn.sortDirection (DaviSortDirection)

DaviSort.id (dynamic)
DaviSort.direction (DaviSortDirection)

It could be:

DaviColumn.dataComparator (DaviDataComparator)
DaviColumn.sort (DaviSort)

DaviSort.columnId (dynamic)
DaviSort.priority (int)
DaviSort.direction (DaviSortDirection)

DaviSort(this.columnId, [this.direction = DaviSortDirection.ascending, this.priority = 1])

With default priority value, it facilitates use in simple sorting.

This priority is important for multi-sorting and would be used in the DaviSort.hashCode. It means that DaviModel.sort method would use the priority and no longer the order of the list.

The priority is required here:

  /// Gets the sorted columns.
  List<DaviColumn<DATA>> get sortedColumns {
    List<DaviColumn<DATA>> list =
        _columns.where((column) => column.isSorted).toList();
    list.sort((a, b) => a.sortPriority!.compareTo(b.sortPriority!));
    return UnmodifiableListView(list);
  }

I liked it.

I have to create some unit tests and validations to ensure that the column.sort doesn't have a different id than the column. But I prefer it to the current possible inconsistency of having null priority and non-null direction for example.

[ykrasik]

I'm excited to see how this turns out :)

About priority - I didn't fully understand why priority is needed for multisort, isn't the order of the DaviSort in the list enough? That would be the priority, no?

[caduandrade]

Hi @ykrasik!

I was finishing the refactoring. I just committed. You're right. During testing, I realized that it would be very strange to define the order in the class and have the order of the list. The list already defines the order in multi-sorting. But DaviSort will still need to have a priority attribute, updated by the model and used by the DaviModel.sortedColumns. This attribute is not used in == and hashCode. Otherwise, it is necessary to have a separate internal list with only the sorted columns and keep them in sync when adding/removing columns.
And while I'm writing I had an idea that maybe with Dart's mixin, it's possible to keep the priority invisible in DaviSort and available only for DaviModel (Dart Hack). I will try.

[ykrasik]

That sounds like something that could run into trouble down the road :) When 2 DaviSort instances unexpectedly report they are equal, even though they have a different priority. Maybe you could store the priority in a Map<DaviSort, int> in the model? It should work with proper == & hashCode on DaviSort. I actually have no idea about the semantics and internals of what you're describing, so just making a blind suggestion.

[caduandrade]

Problem solved. I put the priority in the DaviColumn without the possibility of having it null and the sort not, or vice versa.

1. no second structure to store the priority
2. no attribute in DaviSort
3. the order of the DaviSort list is the order of priorities
4. through the column it is still possible to obtain the priority

There is no perfect solution but I think it is the best. (until you remember some other detail 😄 )

Now it remains to resolve the icon.

[caduandrade]

@ykrasik,

What do you think?

[ykrasik]

That makes sense to me. There does seem to be confusion online. For me, ascending means "a is shown before b" or the smallest element is at the top, which is what those icons convey. Anyway, those arrows don't really suit my app's look so I will probably override them, but the directions seem correct :)

[ykrasik]

Been using the latest version - its clean and easy to use. Little code to maintain on my part. Looks great!

[caduandrade]

When you said that you would need to change the icon, I realized that it makes no sense to change it to another IconData. It would be better to be able to define a Widget to have a raster image if necessary. I'm making a sortIconBuilder.

[caduandrade]

I think this one is done.