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

Feature/define sorting #176

Merged
merged 11 commits into from
Sep 18, 2017
Merged

Feature/define sorting #176

merged 11 commits into from
Sep 18, 2017

Conversation

bjoerge
Copy link
Member

@bjoerge bjoerge commented Sep 15, 2017

This adds support for defining a set of sort options on schema types:

Example:

{
  type: 'object',
  name: 'myType',
  // ...
  sorting: [
    {
      title: 'Title',
      name: 'title',
      orderBy: {
        title: 'asc',
        publicationYear: 'asc'
      }
    }
  ]
  //...
}

These will be options for sorting the list of documents, together with _createdAt and _updatedAt

This also introduces a second argument to preview.prepare called viewOptions. If the document to be previewed is subject to a certain sort order, the sorting will be passed in on viewOptions.sorting. This allows for returning another preview title if the list of documents should be sorted by a field that is not normally visible, e.g. a localized field.

Copy link
Member

@rexxars rexxars left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks solid! A few nitpicks, but otherwise 👍


const OVERRIDABLE_FIELDS = ['jsonType', 'type', 'name', 'title', 'description', 'options', 'inputComponent']
const OVERRIDABLE_FIELDS = ['jsonType', 'sorting', 'type', 'name', 'title', 'description', 'options', 'inputComponent']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for derailing, but should I have added hidden to this list, too?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah! Looks like readOnly is missing there too.

const DEFAULT_SORT_OPTIONS = [
{
title: 'Last edited',
name: '__updatedAt',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not clear to me why these are double-underscored - is this intentional? If so, maybe add a comment explaining why?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right - The double underscores were there to prevent conflicts with possible user-defined sort options with the same name. But on second thought; being able to override the semantics of updatedAt is actually a nice feature that we could document.

: DEFAULT_SORT_OPTIONS).map(option => {
return {
...option,
icon: SortIcon,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we should allow icon to be specified in sort options? It would make sense to allow say a clock/calendar icon for date sorting and a Aa↓ icon for sorting by title, for instance.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totally! Will fix.

Copy link
Contributor

@thomax thomax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a solid implementation of a sorely needed feature. Thumbs up!

@bjoerge
Copy link
Member Author

bjoerge commented Sep 18, 2017

After a few IRL discussions we'd like to do a few minor changes to the proposed format:

  • Change the key from sorting to orderings
  • Change from orderBy on each ordering to by.
  • Change the format of by from an object to an array with field + direction

Updated format:

{
  type: 'object',
  name: 'myType',
  // ...
  orderings: [
    {
      title: 'Title',
      name: 'title',
      by: [
        {field: 'title', direction: 'asc'},
        {field: 'publicationYear', direction: 'desc'}
      ]
    }
  ]
  //...
}

@bjoerge bjoerge merged commit d745a58 into next Sep 18, 2017
@bjoerge bjoerge deleted the feature/define-sorting branch September 18, 2017 12:39
@bjoerge bjoerge mentioned this pull request Sep 19, 2017
bjoerge added a commit that referenced this pull request Sep 19, 2017
* [test-studio] Setup test cases

* [schema] Support 'sorting' on object types + guess default sort config

* [base] Pass view options to preview.prepare

* [base] Expose sort icon

* [desk-tool] Support custom sorting of documents list

* [desk-tool] Remove unused state key

* [desk-tool] Code nits

* [desk-tool] Remove prefixed default sort options and make customizable

* [desk-tool] Make icon configurable

* [schema] Add missing overridable fields

* [chore] More consistent usage of 'ordering'
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

Successfully merging this pull request may close these issues.

3 participants