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

feat(row): added a row renderer via plugin #115

Merged
merged 3 commits into from
Feb 19, 2017
Merged

feat(row): added a row renderer via plugin #115

merged 3 commits into from
Feb 19, 2017

Conversation

darlenya
Copy link
Contributor

Hi Benjamin,

now the renderer is called via plugin method. I also started to add a test for it. But the row.test.js is not called and it is the first time I work with react and had a few problems to fix some lint errors in the test. Currently the test is commented Row.test.js in line 270.

@codecov-io
Copy link

codecov-io commented Feb 19, 2017

Codecov Report

Merging #115 into master will increase coverage by 0.15%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #115      +/-   ##
==========================================
+ Coverage   77.08%   77.23%   +0.15%     
==========================================
  Files         112      112              
  Lines        2736     2728       -8     
  Branches      273      276       +3     
==========================================
- Hits         2109     2107       -2     
+ Misses        416      412       -4     
+ Partials      211      209       -2
Impacted Files Coverage Δ
src/components/layout/table-row/Row.jsx 50% <100%> (+2.43%)
src/util/stateGetter.js 94.73% <ø> (-0.27%)
src/util/lastUpdate.js 92.85% <ø> (+7.14%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0f78c3a...71d1bbb. Read the comment docs.

Copy link
Owner

@bencripps bencripps left a comment

Choose a reason for hiding this comment

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

Everything looks great. A few minor changes for docs, and add the test I've provided in the review, and we're ready to merge!

```
| Prop|Type|Description|
|:---------|:------------|:--------------|
| enabled|bool|whether the bulk action toolbar should be used|
Copy link
Owner

Choose a reason for hiding this comment

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

Only need to list enabled and renderer here. I will add a new doc to discuss this feature in more detail.

@@ -350,6 +350,31 @@ export const plugins = {
| enabled|bool|whether the bulk action toolbar should be used|
| actions|arrayOf(object)|the actions (including button text, and event handler) that will be displayed in the bar|


## Row renderer
Copy link
Owner

Choose a reason for hiding this comment

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

Row Renderer

let rowEl;

if (isPluginEnabled(plugins, 'ROW') &&
typeof plugins.ROW.renderer === 'function') {
Copy link
Owner

Choose a reason for hiding this comment

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

👍

@@ -267,6 +267,45 @@ describe('The Grid Row Component', () => {

});

// it('Should render a row with a custom row renderer', () => {
Copy link
Owner

Choose a reason for hiding this comment

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

Replace your commented out test with the following:

  it('Should render a row with a custom rowRenderer', () => {
        const dynamicEditProps = {
            ...props,
            row: fromJS({
                name: 'Michael Jordan',
                position: 'Shooting Guard',
                _key: 'row-0',
                header: true
            }),
            plugins: {
                ROW: {
                    enabled: true,
                    /* eslint-disable react/prop-types */
                    renderer: ({ rowProps, cells }) => {
                        rowProps.className = `${rowProps.className} custom`;
                        return (
                            <tr { ...rowProps}>
                                { cells }
                            </tr>
                        );
                    }
                    /* eslint-enable react/prop-types */
                }
            }
        };

        const component = mount(<Row { ...dynamicEditProps } />);

        expect(
            component.first('tr').hasClass('custom')
        ).toEqual(
            true
        );

        expect(
            component.first('tr').hasClass('react-grid-row')
        ).toEqual(
            true
        );
    });

@bencripps bencripps merged commit b8b57a6 into bencripps:master Feb 19, 2017
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.

None yet

3 participants