Skip to content

Add support for nested grid reducers #106

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

Merged
merged 2 commits into from
Feb 17, 2017

Conversation

drownbes
Copy link
Contributor

reducerKeys now could be string that means
that root grid reducers is on the state[reducersKeys]
position in app state

reducerKeys now could be string that means
that root grid reducers is on the state[reducersKeys]
position in app state
@codecov-io
Copy link

codecov-io commented Feb 16, 2017

Codecov Report

Merging #106 into master will decrease coverage by -0.04%.
The diff coverage is 82.35%.

@@            Coverage Diff             @@
##           master     #106      +/-   ##
==========================================
- Coverage   77.19%   77.15%   -0.04%     
==========================================
  Files         112      112              
  Lines        2727     2732       +5     
  Branches      280      270      -10     
==========================================
+ Hits         2105     2108       +3     
- Misses        412      416       +4     
+ Partials      210      208       -2
Impacted Files Coverage Δ
src/components/layout/FixedHeader.jsx 48.92% <ø> (-2.88%)
src/components/plugins/editor/Inline.jsx 85.54% <ø> (ø)
src/components/Grid.jsx 77.77% <ø> (ø)
src/components/plugins/selection/CheckBox.jsx 54.76% <ø> (ø)
src/components/layout/TableRow.jsx 66.66% <ø> (+0.95%)
...rc/components/plugins/gridactions/ActionColumn.jsx 52.75% <ø> (ø)
...mponents/plugins/gridactions/actioncolumn/Menu.jsx 68.75% <ø> (ø)
src/components/layout/Header.jsx 63.88% <ø> (-1.83%)
src/components/layout/table-row/Row.jsx 47.56% <ø> (+2.05%)
src/util/stateGetter.js 95% <100%> (+0.88%)
... and 6 more

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 7f40d71...6a3803e. Read the comment docs.

@bencripps
Copy link
Owner

bencripps commented Feb 16, 2017

Thanks for the PR for #95!

Could you give a brief description of how this changes the API for grid? Could you provide examples of how you would instantiate grid -- and does it still work both ways?

This version still works, correct?

import { combineReducers } from 'redux';
import { Reducers as gridReducers } from 'react-redux-grid';

import myAppReducer from './customReducers/app';
import myDataReducer from './customReducers/data';

export const rootReducer = combineReducers({
    myAppReducer,
    myDataReducer,
    ...gridReducers
});

How do you envision people using this new method?

@bencripps bencripps self-requested a review February 16, 2017 16:44
@bencripps bencripps self-assigned this Feb 16, 2017
&& props.reducerKeys[key]) {
if (props && props.reducerKeys) {
if (typeof props.reducerKeys === 'string' &&
props.reducerKeys.length > 0) {
Copy link
Owner

Choose a reason for hiding this comment

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

if reducerKeys is of type string and it's defined, won't it have a length > 0? Seems like even if you pass the reducerKey of grid, this check would return false?

Added example to the docs
Removed redundant check
@bencripps
Copy link
Owner

New changes look good. I will test this out tonight. Thanks!

@bencripps
Copy link
Owner

Changes look good, tests look good, and I've verified it works locally. Would you mind squashing your two commits into one?

Great PR!

@drownbes
Copy link
Contributor Author

@bencripps As I know github have added squash on merge for pull request, aren't they?
https://github.com/blog/2141-squash-your-commits

@bencripps bencripps merged commit b87973f into bencripps:master Feb 17, 2017
@bencripps
Copy link
Owner

That's an awesome feature I didn't know about! Squashed & merged!

bencripps pushed a commit that referenced this pull request Feb 17, 2017
* Add support for nested grid reducers
* Added example to the docs
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