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

fix #1151 fixed some warnings #1226

Merged
merged 3 commits into from
Nov 2, 2016

Conversation

saidaipparla
Copy link
Contributor

No description provided.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 79.765% when pulling 7a09320 on saidaipparla:remove_warning into 27a45b0 on geosolutions-it:master.

Copy link
Contributor

@mbarto mbarto left a comment

Choose a reason for hiding this comment

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

Are there any remaining warnings?

@@ -90,7 +90,7 @@ var MapGrid = React.createClass({
renderMetadataModal() {
if (this.props.metadataModal) {
let MetadataModal = this.props.metadataModal;
return (<MetadataModal ref="metadataModal" show={this.props.currentMap && this.props.currentMap.displayMetadataEdit} onHide={this.props.resetCurrentMap}
return (<MetadataModal key="metadataModal" show={this.props.currentMap && this.props.currentMap.displayMetadataEdit} onHide={this.props.resetCurrentMap}
Copy link
Contributor

Choose a reason for hiding this comment

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

You cannot remove the ref, it's used

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok i will keep ref and add key

@@ -17,15 +17,15 @@ const CreateNewMap = React.createClass({
mapType: React.PropTypes.string,
onGoToMap: React.PropTypes.func,
colProps: React.PropTypes.object,
isLoggedIn: React.PropTypes.bool
isLoggedIn: React.PropTypes.object
Copy link
Contributor

Choose a reason for hiding this comment

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

bool is correct. You need to change the connect to set a bool value

},
contextTypes: {
router: React.PropTypes.object
},
getDefaultProps() {
return {
mapType: "leaflet",
isLoggedIn: false,
isLoggedIn: "",
Copy link
Contributor

Choose a reason for hiding this comment

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

false is correct

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok i will change this

@saidaipparla
Copy link
Contributor Author

yes as far i have seen there are other two warnings

  • when we will click on print link from menu
  • when we will click on settings icon in layer properties

@mbarto
Copy link
Contributor

mbarto commented Oct 28, 2016

Ok, then fix also the two remaining warnings

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.09%) to 79.672% when pulling 5dbb0a7 on saidaipparla:remove_warning into 27a45b0 on geosolutions-it:master.

@mbarto
Copy link
Contributor

mbarto commented Oct 28, 2016

Is the "when we will click on settings icon in layer properties" still there?

@@ -51,6 +51,6 @@ const CreateNewMap = React.createClass({
module.exports = {
CreateNewMapPlugin: connect((state) => ({
mapType: (state.maps && state.maps.mapType) || (state.home && state.home.mapType),
isLoggedIn: state && state.security && state.security.user
isLoggedIn: state && state.security && state.security.user && state.security.user.enabled
Copy link
Contributor

Choose a reason for hiding this comment

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

isLoggedIn: state && state.security && state.security.user && true || false

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok i will change

@@ -385,7 +385,7 @@
"closeGlyph": "1-close",
"submitConfig": {
"buttonConfig": {
"bsSize": "medium",
"bsSize": "small",
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you checked that the button doesn't change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes i checked its changed

@coveralls
Copy link

Coverage Status

Coverage increased (+0.005%) to 79.77% when pulling 0effe65 on saidaipparla:remove_warning into 27a45b0 on geosolutions-it:master.

@mbarto mbarto merged commit add5fe5 into geosolutions-it:master Nov 2, 2016
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.

3 participants