Skip to content

Conversation

@chunming-c
Copy link
Member

fix #126 and #127

Hi @AllenFang, the PR aims to support sorted classes and style. Please take a look, thanks :)

  • support sorted classes and style.
  • typo fix

@AllenFang
Copy link
Member

AllenFang commented Nov 5, 2017

@Chun-MingChen I just wondering why you have following design:

sortedHeader: {
  classes: ....
  styles: ....
}

Our cell, row, selection and cell edit always separate classes and styles as two different prop

@chunming-c chunming-c force-pushed the development/sorted-classes-n-style branch 2 times, most recently from d9be2c2 to 7280e3c Compare November 5, 2017 09:27
@chunming-c
Copy link
Member Author

@AllenFang

Sure it does be inconsistent. I've fixed it to separate sortingClass and sorting style into two props. Thanks 👍

docs/README.md Outdated

### <a name='sortingHeaderStyle'>sortingHeaderStyle - [Object | Function]</a>

It's similiar to `sortingHeaderClasses`. It allows to customize style of `header cell` which is sorting based on. `Object` and `callback` function are acceptable. `callback` takes `2` arguments and an `Object` is expected to return:
Copy link
Member

Choose a reason for hiding this comment

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

An style object or callback function are acceptable

docs/README.md Outdated
Furthermore, it also accepts a callback function which takes `2` arguments and `String` is expected to return:

```js
const sortingHeaderClasses = function callback(column, colIndex) { ... }
Copy link
Member

Choose a reason for hiding this comment

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

es6 style

docs/README.md Outdated
```

* column: The value of current column.
* colIndex: The index of the current column being processed in BootstrapTable.
Copy link
Member

Choose a reason for hiding this comment

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

Additional pass sort order to callback function as third argument

@chunming-c chunming-c force-pushed the development/sorted-classes-n-style branch from 7280e3c to af095c0 Compare November 25, 2017 10:38
@chunming-c chunming-c force-pushed the development/sorted-classes-n-style branch from f438426 to 1014de2 Compare November 25, 2017 10:50
@chunming-c
Copy link
Member Author

Hi @AllenFang:

Sorry for commit late. After our discussion, I implemented header sorting classes and style in column level. Furthermore, isLastSorting indicates that the field was last sorting or not. Please take a look, thanks :)

Copy link
Member

@AllenFang AllenFang left a comment

Choose a reason for hiding this comment

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

good job, but something need to change.

docs/columns.md Outdated
### <a name='headerSortingClasses'>headerSortingClasses - [String | Function]</a>

`headerSortingClasses` allows to customize `class` for header cell which the table was sorting based on.
Copy link
Member

Choose a reason for hiding this comment

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

headerSortingClasses allows to customize class for header cell when this column is sorting

docs/columns.md Outdated

* `column`: The value of current column.
* `sortOrder`: The order of current sorting
* `isLastSorting`: Is the last one of sorting.
Copy link
Member

Choose a reason for hiding this comment

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

Is the last one of sorted columns.

docs/columns.md Outdated

### <a name='headerSortingStyle'>headerSortingStyle - [Object | Function]</a>

It's similiar to [headerSortingClasses](#headerSortingClasses). It allows to customize the style of header cell which the table was sorting based on. A style `Object` and `callback` are acceptable. `callback` takes **4** arguments and an `Object` is expected to return:
Copy link
Member

Choose a reason for hiding this comment

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

It allows to customize the style of header cell when this column is sorting

data={ products }
columns={ columns }
defaultSorted={ defaultSorted }
/>
Copy link
Member

Choose a reason for hiding this comment

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

Thanks

.add('Default Sort Table', () => <DefaultSortTable />)
.add('Custom Sort Fuction', () => <CustomSortTable />);
.add('Custom Sort Fuction', () => <CustomSortTable />)
.add('Header sorting classes', () => <HeaderSortingClassesTable />)
Copy link
Member

Choose a reason for hiding this comment

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

Custom Classes on Sorting Header Column

.add('Custom Sort Fuction', () => <CustomSortTable />);
.add('Custom Sort Fuction', () => <CustomSortTable />)
.add('Header sorting classes', () => <HeaderSortingClassesTable />)
.add('Header sorting style', () => <HeaderSortingStyleTable />);
Copy link
Member

Choose a reason for hiding this comment

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

Custom Style on Sorting Header Column

});
});

describe('when headerSortingClasses was defined ', () => {
Copy link
Member

Choose a reason for hiding this comment

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

when headerSortingClasses is defined

/>);
});

it('should append classes correcly', () => {
Copy link
Member

Choose a reason for hiding this comment

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

should + V-ing

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought that will be great for should + VR :)

});
});

describe('if column.headerClasses was defined as well', () => {
Copy link
Member

Choose a reason for hiding this comment

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

if column.headerClasses is defined as well

});
});

describe('when headerSortingStyle was defined', () => {
Copy link
Member

Choose a reason for hiding this comment

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

is

@chunming-c chunming-c force-pushed the development/sorted-classes-n-style branch from 5092908 to 90cada4 Compare December 9, 2017 10:06
@AllenFang AllenFang merged commit 00f1105 into develop Dec 10, 2017
@AllenFang AllenFang deleted the development/sorted-classes-n-style branch December 10, 2017 05:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants