-
Notifications
You must be signed in to change notification settings - Fork 1.7k
get certifications from BadgeReg, show them in accounts overview #3768
Changes from all commits
2b34d76
b32b636
409c4ad
e1c5796
5862f2a
a84cd91
e536290
3dc943e
fd88421
6d20592
bc33231
d67a938
240bab2
afba259
f59f7c5
4c42ded
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,17 +17,27 @@ | |
const initialState = {}; | ||
|
||
export default (state = initialState, action) => { | ||
if (action.type !== 'addCertification') { | ||
return state; | ||
if (action.type === 'addCertification') { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. switch statement would be good here as well, since we are only checking the type, default is the state return. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changing it to a switch statement in this case means that the variable declarations aren't as local anymore. The linter will complain. Moving them up is ugly because we're assigning stuff we may not need. |
||
const { address, id, name, icon, title } = action; | ||
const certifications = state[address] || []; | ||
|
||
if (certifications.some((c) => c.id === id)) { | ||
return state; | ||
} | ||
|
||
const newCertifications = certifications.concat({ | ||
id, name, icon, title | ||
}); | ||
return { ...state, [address]: newCertifications }; | ||
} | ||
|
||
const { address, name, icon, title } = action; | ||
const certifications = state[address] || []; | ||
if (action.type === 'removeCertification') { | ||
const { address, id } = action; | ||
const certifications = state[address] || []; | ||
|
||
if (certifications.some((c) => c.name === name)) { | ||
return state; | ||
const newCertifications = certifications.filter((c) => c.id !== id); | ||
return { ...state, [address]: newCertifications }; | ||
} | ||
const newCertifications = certifications.concat({ name, icon, title }); | ||
|
||
return { ...state, [address]: newCertifications }; | ||
return state; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,10 +16,8 @@ | |
|
||
import React, { Component, PropTypes } from 'react'; | ||
import { connect } from 'react-redux'; | ||
import { bindActionCreators } from 'redux'; | ||
|
||
import { hashToImageUrl } from '~/redux/providers/imagesReducer'; | ||
import { fetchCertifications } from '~/redux/providers/certifications/actions'; | ||
|
||
import defaultIcon from '../../../assets/images/certifications/unknown.svg'; | ||
|
||
|
@@ -29,14 +27,7 @@ class Certifications extends Component { | |
static propTypes = { | ||
account: PropTypes.string.isRequired, | ||
certifications: PropTypes.array.isRequired, | ||
dappsUrl: PropTypes.string.isRequired, | ||
|
||
fetchCertifications: PropTypes.func.isRequired | ||
} | ||
|
||
componentWillMount () { | ||
const { account, fetchCertifications } = this.props; | ||
fetchCertifications(account); | ||
dappsUrl: PropTypes.string.isRequired | ||
} | ||
|
||
render () { | ||
|
@@ -73,15 +64,13 @@ function mapStateToProps (_, initProps) { | |
|
||
return (state) => { | ||
const certifications = state.certifications[account] || []; | ||
return { certifications }; | ||
}; | ||
} | ||
const dappsUrl = state.api.dappsUrl; | ||
|
||
function mapDispatchToProps (dispatch) { | ||
return bindActionCreators({ fetchCertifications }, dispatch); | ||
return { certifications, dappsUrl }; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Less squashing, break would be good before the return line. |
||
}; | ||
} | ||
|
||
export default connect( | ||
mapStateToProps, | ||
mapDispatchToProps | ||
null | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could remove completely There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I actually like |
||
)(Certifications); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really picky, but would prefer a line before this block, just indicates a break between assigning and doing. (Not a crisis, just aids readability once again)