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

Changes are made to insert a new row into a specific location of grid by passing rowIndex parameter to addNewRow(ADD_NEW_ROW) action. #129

Merged
merged 2 commits into from
Mar 2, 2017

Conversation

underwater222
Copy link
Contributor

I have test it to add a new row into a specific location and confirmed that it works. However it fails some testing on 'npm t'. Please take a look.

passing rowIndex parameter to addNewRow(ADD_NEW_ROW) action.
@codecov-io
Copy link

codecov-io commented Feb 27, 2017

Codecov Report

Merging #129 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #129      +/-   ##
==========================================
+ Coverage   81.33%   81.34%   +<.01%     
==========================================
  Files         111      111              
  Lines        4490     4492       +2     
==========================================
+ Hits         3652     3654       +2     
  Misses        838      838
Impacted Files Coverage Δ
src/actions/plugins/editor/EditorActions.js 88.23% <ø> (ø)
src/reducers/actionHelpers/datasource.js 65.24% <100%> (+0.42%)

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 5055622...dd908d7. 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.

Make sure to run npm run lint, I've made a comment on one stylistic issue but everything else looks great!

@@ -160,7 +160,8 @@ export const addNewRow = (state, { rowId, stateKey }) => {
data = new List();
}

const newData = data.unshift(newRow);
rowIndex = rowIndex===undefined?0:rowIndex;
Copy link
Owner

Choose a reason for hiding this comment

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

rowIndex = rowIndex === undefined
    ? 0
    : rowIndex;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Greate! Thanks.

Copy link
Owner

Choose a reason for hiding this comment

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

No problem, once you push the change, I will merge!

@underwater222
Copy link
Contributor Author

I have push the coding style change for "? :" operator . Do I need to create a new pull request or can you merge from this pull request on my "addnewrowatpos" branch?

@bencripps bencripps merged commit 5695cfd into bencripps:master Mar 2, 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