If the data is not sorted by the same field that creates the groups, multiple groups will be created across pages (see video below). It sounds like we should use the groupByField for sorting if it is present. "group by" is a higher-level "sort by" that also groups the data visually — sort is contained in groupBy. How do we surface this at the UI level?

Size Change: +351 B (+0.02%)

Total Size: 1.89 MB

_{compressed-size-action}

"group by" is a higher-level "sort by" that also groups the data visually — sort is contained in groupBy. How do we surface this at the UI level?

Considering this thought, there are two potential implementations. The current implementation adds a new property to the view:

{
  type: 'grid',
  sort: {
    field: 'fieldIdSort'
    direction: 'asc'
  },
  groupByField: 'fieldIdGroupBy'
}

But we could also consider:

{
  type: 'grid',
  sort: {
    field: 'fieldIdSort'
    direction: 'asc',
    grouped: true
  }
}

In the second suggestion, I'd expect the field used to sort the data request to be sort.field, and the UI for grouping to be part of the sort affordance. In the first one, we need to decide if the data request is sorted by sort.field or groupByField, and the UI for grouping can be independent of sorting.

Consider the relationship with fields, titleField, etc.

It doesn't look like we need to do anything here. Consumers can do this if that makes sense to them:

{
  fields: [ 'someField', 'groupByField' ],
  groupByField: 'groupByField'
}

It's the same that happens with titleField or mediaField. I don't see any use case where consumers would want to display twice the title, media, or groupBy fields. We don't have any guardrails for title & media that prevent them from being used in the fields array, so none should be added for groupBy either in this PR.

"group by" is a higher-level "sort by" that also groups the data visually — sort is contained in groupBy. How do we surface this at the UI level?

For me that is something that the DataViews package should actually do internally. Have some kind of function that takes the "data" and group it no?

I do see a need to translate a view to a REST API query when there's grouping applied (and in this case, it might mean sorting) but this is a bit separate from the rendering itself. I expect the DataViews to group things properly if it receives the Data not in the right order.

okay, I've updated the code to use this approach instead:

{
  type: 'grid',
  sort: {
    field: 'fieldIdSort'
    direction: 'asc',
    grouped: true
  }
}

Also prepended the field label, so values that aren't self-explanatory work better (boolean, numbers):

What do you think about my previous commentS?

What do you think about my previous commentS?

Sorry, GitHub didn't display them to me when I posted the update. Commenting now.

I've been looking at other examples:

Finder (MacOS)	Google Drive

• Finder: sorting is applied within the group. Groups remain in the same position (they have an order set by MacOs and can't be ordered by the user).
• Google Drive: the group acts as a sorting mechanism ("Recent screen").
• In both cases, the group is not exactly the field. For example, Google Drive docs have a date, but the grouping is human-understandable dates (yesterday, last month, earlier this year). Finder does the same in some categories (sizes are grouped by ranges). This is interesting and something that can be a follow-up (added in the description): for example, fields could provide a custom groupBy function.

The way Finder works seems the more versatile for our needs, and so making groupBy part of the sorting doesn't help to implement that. I'll revert that commit and will go back to the first approach using a separate prop.

One other aspect to consider is data fetching. The Finder behavior works by sorting the data twice: first by group, secondly by the "sort by" element. It has all the data available to mangle it. The same expectation holds true for the DataViews component: it should receive the data sorted (by group, and then by sort), otherwise we'll run into behaviors like these.

There are two things here:

• The filterSortAndPaginate utility needs to implement that (sort by "group by", then by "sort by"). I'll do that in this PR.
• Requests to the WordPress REST Endpoints. I understand the REST endpoints don't support sorting by multiple fields (see). I'm not sure how feasible it is to implement that, but I consider it separate from this PR. One option to circumvent that limitation is to make the request using the "group by" as the order. Then, client-side, we'll sort within each group with the "sort by" key. This logic is the consumer responsibility, not the DataViews component's, which is agnostic about WordPress REST endpoints.

I'll look at updating this PR tomorrow.

I'm not sure how feasible it is to implement that, but I consider it separate from this PR. One option to circumvent that limitation is to make the request using the "group by" as the order. Then, client-side, we'll sort within each group with the "sort by" key. This logic is the consumer responsibility, not the DataViews component's, which is agnostic about WordPress REST endpoints.

From my perspective, that's a request issue and not a DataViews issue. In other words, we shouldn't be adapting the DataViews implementation to the REST API, instead we should update the REST API endpoint to support either "group by" or "multiple sorts"

I don't think sorting in the client after sorting with the REST API works though, it gives broken results when you have pagination.

From my perspective, that's a request issue and not a DataViews issue.

Yeah, agreed. This needs to be looked at separately at the consumer level.

Pushed the changes and this is how it works now:

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: oandregal <oandregal@git.wordpress.org>
Co-authored-by: youknowriad <youknowriad@git.wordpress.org>
Co-authored-by: tyxla <tyxla@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

This is looking good, I wonder how much of this is "layout specific". Like what if this is done at a higher level than that layout, would that work? Maybe not

This depends on the layout. For example, for grid, perhaps it could have worked as something higher-level, but not for table, for example (we want the groups within the table body).

I wonder if we intend to use this with the table view as well. I think it could work nicely. Maybe not something for this PR though.

That's the idea, it's part of the follow-ups. Responded there, but I'd think the original issue may need a design refresh.

Changelog needs updating?

🤦 It does! Done at 4aef87a