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

modified renderButtons() in drawer/Menu.jsx (issue 1526) #1527

Merged
merged 6 commits into from
Mar 10, 2017

Conversation

diegosanmartino
Copy link
Contributor

config's ex:
"name": "Measure",
"override": {
"DrawerMenu": {
"buttonConfig": {
"tooltip": "Measure",
"buttonClassName": "square-button no-border"
},
"glyph": "1-stilo",
"position": 4
}
}

@simboss simboss added the ready label Mar 7, 2017
{child.props.glyph ? <Glyphicon glyph={child.props.glyph} /> : child.props.icon}
</Button>
));
let toReturn = [];
Copy link
Contributor

Choose a reason for hiding this comment

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

Please, don't do sideeffects, the map function on arrays returns an array, don't use it as a forEach.

Something more similar to this is preferrable:

return this.props.children.map((child) => {
    const button = (<Button key={child.props.eventKey} bsSize="large" className={(child.props.buttonConfig && child.props.buttonConfig.buttonClassName) ? child.props.buttonConfig.buttonClassName : "square-button"} onClick={this.props.onChoose.bind(null, child.props.eventKey)} bsStyle={this.props.activeKey === child.props.eventKey ? 'default' : 'primary'}>
                        {child.props.glyph ? <Glyphicon glyph={child.props.glyph} /> : child.props.icon}
                    </Button>);
    const tooltip = <Tooltip key={"tooltip." + child.props.eventKey} id={"tooltip." + child.props.eventKey}>{child.props.tooltip || ''}</Tooltip>;
    return child.props.tooltip ? (
        <OverlayTrigger placement={"top"} key={"overlay-trigger." + child.props.eventKey}
            overlay={tooltip}>
            {button}
        </OverlayTrigger>
    ) : button;
});

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

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 77.092% when pulling f718171 on diegosanmartino:1526 into 6f57ada on geosolutions-it:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 77.122% when pulling f441ee4 on diegosanmartino:1526 into 6f57ada on geosolutions-it:master.

@@ -6,7 +6,7 @@
* LICENSE file in the root directory of this source tree.
*/
var React = require('react');
var {Glyphicon, Button} = require('react-bootstrap');
var {Glyphicon, Button, OverlayTrigger, Tooltip} = require('react-bootstrap');
var Sidebar = require('react-sidebar').default;

Copy link
Contributor

Choose a reason for hiding this comment

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

var Message = require('../../components/I18N/Message');

const button = (<Button key={child.props.eventKey} bsSize="large" className={(child.props.buttonConfig && child.props.buttonConfig.buttonClassName) ? child.props.buttonConfig.buttonClassName : "square-button"} onClick={this.props.onChoose.bind(null, child.props.eventKey)} bsStyle={this.props.activeKey === child.props.eventKey ? 'default' : 'primary'}>
{child.props.glyph ? <Glyphicon glyph={child.props.glyph} /> : child.props.icon}
</Button>);
const tooltip = <Tooltip key={"tooltip." + child.props.eventKey} id={"tooltip." + child.props.eventKey}>{child.props.buttonConfig.tooltip || ''}</Tooltip>;
Copy link
Contributor

Choose a reason for hiding this comment

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

const tooltip = <Tooltip key={"tooltip." + child.props.eventKey} id={"tooltip." + child.props.eventKey}><Message msgId={child.props.buttonConfig.tooltip}/></Tooltip>;

</Button>);
const tooltip = <Tooltip key={"tooltip." + child.props.eventKey} id={"tooltip." + child.props.eventKey}>{child.props.buttonConfig.tooltip || ''}</Tooltip>;
return child.props.buttonConfig.tooltip ? (
<OverlayTrigger placement={"top"} key={"overlay-trigger." + child.props.eventKey}
Copy link
Contributor

Choose a reason for hiding this comment

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

bottom, if you put it at top, it's not visible (did you test it?)

Copy link
Contributor Author

@diegosanmartino diegosanmartino Mar 8, 2017

Choose a reason for hiding this comment

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

const tooltip = <Tooltip key={"tooltip." + child.props.eventKey} id={"tooltip." + child.props.eventKey}>;

child.props.buttonConfig.tooltip is correct but the tooltip is empty

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you have the string in your translations file?

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

@coveralls
Copy link

Coverage Status

Coverage remained the same at 77.122% when pulling 7f4b130 on diegosanmartino:1526 into 6f57ada on geosolutions-it:master.

@MV88
Copy link
Contributor

MV88 commented Mar 9, 2017

Hi @diegosanmartino I would suggest you to use the Atom editor (or Sublime) with the plugins suggested here https://geosolutions-it.github.io/MapStore2/developer-guide/building-and-developing (especially eslint) in order to see in the editor some syntax error. This would avoid to push commit that will fail the travis build. Thank you

@mbarto mbarto merged commit 679ca32 into geosolutions-it:master Mar 10, 2017
@mbarto mbarto removed the ready label Mar 10, 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.

5 participants