zetkin / app.zetkin.org

Revamped Zetkin web interface.
https://app-zetkin-org.vercel.app
23 stars 40 forks source link

Rows that have no value are sorted on top in ascending order #1511

Open kaulfield23 opened 10 months ago

kaulfield23 commented 10 months ago

Please be as thorough as possible, but if you can't answer everything, please just submit what you have! Thank you!

Description

When sorting happens in view column, normally it is expected to see values at the top of the list in ascending or descending order. However, empty cells are prioritized over cells that have values in view. Therefore, it can be inconvenient to scroll down to see the values in ascending order.

Steps to reproduce

  1. Go to view
  2. Add column Notes (or any other columns)
  3. Give values to each cells. Make sure to leave some cells empty.
  4. Sort the Notes column.

Expected Behaviour

The sorting should show values first before empty cells. So the order should be "values - null(empty cells)" in ascending / descending order in all type of columns.

Actual Behaviour

It has this following order.

Screenshots (if you have any)

Ascending order image

Descending order image

Bagera commented 1 month ago

I think null/empty cells should be disregarded in the ordering and added at the bottom no matter the direction. No more design is needed in my opinion.

henrycatalinismith commented 3 days ago

There's an example in the Data Grid docs that demonstrates how to achieve this exact sorting behavior.

The Data Grid considers the sortComparator function symmetric, automatically reversing the return value for descending sorting by multiplying it by -1.

While this is sufficient for most use cases, it is possible to define an asymmetric comparator using the getSortComparator function – it receives the sorting direction as an argument and returns a comparator function.

In the demo below, the getSortComparator function is used in the "Quantity" column to keep the null values at the bottom when sorting is applied (regardless of the sorting direction):

https://mui.com/x/react-data-grid/sorting/#asymmetric-comparator

The implementation then showcases a getSortComparator prop being added to the GridColDef.

getSortComparator: (sortDirection) => {
  const modifier = sortDirection === 'desc' ? -1 : 1;
  return (value1, value2, cellParams1, cellParams2) => {
    if (value1 === null) {
      return 1;
    }
    if (value2 === null) {
      return -1;
    }
    return (
      modifier *
      gridStringOrNumberComparator(value1, value2, cellParams1, cellParams2)
    );
  };
},

I've looked at adding this custom comparator. It will be relatively straightforward to add this to useConfigurableDataGridColumns.

The blocker here is that we're using a @mui/x-data-grid-pro version from 2023 (6.14.0), and the PR adding getSortComparator happened in early 2024. It looks like we need to upgrade this dependency to at least 7.0.0 in order to use this.

The notes on the migration from v6 to v7 are interesting enough that it probably deserves to be an issue of its own. I found the PR where the upgrade to v6 happened and it looks like that one took a few weeks to tame!