taiga-ui icon indicating copy to clipboard operation
taiga-ui copied to clipboard

🚀 - Sort direction parameter

Open evantrimboli opened this issue 2 years ago • 1 comments

Which @taiga-ui/* package(s) are relevant/releated to the feature request?

addon-table

Description

Currently, when specifying the direction to sort, you need to pass either -1 | 1. At first glance, this seems less clear than providing an explicit direction, ie. asc | desc.

My first assumption was that since it's supposed to the direction, then -1 would refer to asc since the smaller values would come first:

const makeSort1 = (direction) =>
  (a, b) => direction === -1 ? a.localeCompare(b) : b.localeCompare(a);

However it seems as though the direction is the multiplier applied to the underlying sort, so 1 is ascending:

const makeSort2 = (direction) =>
  (a, b) => a.localeCompare(b) * direction;

Given the second interpretation, this makes sense, however I think it would be clear to use asc | desc explicitly. From an intent perspective, I feel like this would make the API easier to use.

I realize this isn't a huge deal and also would be a breaking change, but I would appreciate your consideration on this issue.

evantrimboli avatar Aug 03 '22 03:08 evantrimboli

No need for a breaking change, it can be an additional directive:

export class HumanReadableDirection {
  @Input()
  set directionName(name: 'asc' | 'desc') {
    this.table.direction = name === 'asc' ? 1 : -1;
  }

  @Output()
  readonly directionNameChange = this.table.directionChange.pipe(
    map(dir => dir === 1 ? 'asc' : 'desc'),
  );

  constructor(private readonly table: TuiTableDirective) {}
}

Just need to think of a proper name :D I'll keep it as a Contributions welcome ticket since it's pretty straightforward and not an essential thing for Taiga UI.

waterplea avatar Aug 04 '22 09:08 waterplea