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

MousePosition degrees template via plugin props #2625

Closed

Conversation

riaanvddool
Copy link
Contributor

@riaanvddool riaanvddool commented Feb 19, 2018

Description

Allow MousePosition degrees template to be selected via MousePosition plugin props

Issues

Please check if the PR fulfills these requirements

What kind of change does this PR introduce? (check one with "x", remove the others)

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

What is the current behavior? (You can also link to an open issue here)
The default for MousePosition plugin is to display coordinates in DMS format but I need DM.
It is not possible to configure the degreeTemplate from configuration but you can create your own MousPosition Plugin and pass your own template or one from the available ones.

What is the new behavior?
The MousePosition degreesTemplate can now be specified in localConfig.json:

{
          "name": "MousePosition",
          "cfg": {
            "editCRS": true,
            "showLabels": true,
            "showToggle": true,
            "degreesTemplateStr": "MousePositionLabelDM",
            "filterAllowedCRS": ["EPSG:4326", "EPSG:3857"],
            "additionalCRS": {}
}

Does this PR introduce a breaking change? (check one with "x", remove the other)

  • Yes
  • No

If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...

Other information:
Discussion: https://groups.google.com/forum/#!topic/mapstore-developers/buz2kpzo09I

render() {

const degreesTemplateComponent = ((degreesTemplateStr) => {
switch( degreesTemplateStr ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

move this function outside render() and remove application of parameter
like so

getDegreesTemplateComponent = (degreesTemplateStr) => {
        switch (degreesTemplateStr) {
            case "MousePositionLabelDD" : return require('../components/mapcontrols/mouseposition/MousePositionLabelDD');
            case "MousePositionLabelDM" : return require('../components/mapcontrols/mouseposition/MousePositionLabelDM');
            case "MousePositionLabelDMS" : return require('../components/mapcontrols/mouseposition/MousePositionLabelDMS');
            case "MousePositionLabelDMSNW" : return require('../components/mapcontrols/mouseposition/MousePositionLabelDMSNW');
            default: return require('../components/mapcontrols/mouseposition/MousePositionLabelDMS');
        }
    };

inside render

let degreesTemplateComponent = this.getDegreesTemplateComponent(this.props.degreesTemplateStr);
        return (
            <MousePositionComponent toggle={<MousePositionButton/>} degreesTemplate={degreesTemplateComponent} {...this.props}/>
        );

switch( degreesTemplateStr ) {
case "MousePositionLabelDD" : return require('../components/mapcontrols/mouseposition/MousePositionLabelDD');
case "MousePositionLabelDM" : return require('../components/mapcontrols/mouseposition/MousePositionLabelDM');
case "MousePositionLabelDMS" : return require('../components/mapcontrols/mouseposition/MousePositionLabelDMS');
Copy link
Contributor

Choose a reason for hiding this comment

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

use 4 space indentation

class MousePosition extends React.Component {

Copy link
Contributor

@MV88 MV88 Feb 19, 2018

Choose a reason for hiding this comment

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

add property degreesTemplateStr with a default (this will be the one that will be overwritten from localConfig

static propTypes = {
        degreesTemplateStr: PropTypes.string
    };
    static defaultProps = {
        degreesTemplateStr: "MousePositionLabelDMS"
    };

also add :
const PropTypes = require('prop-types');

return (
<MousePositionComponent toggle={<MousePositionButton/>} {...this.props}/>
<MousePositionComponent toggle={<MousePositionButton/>} degreesTemplate={degreesTemplateComponent} {...this.props}/>
);
Copy link
Contributor

@MV88 MV88 Feb 19, 2018

Choose a reason for hiding this comment

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

You can add a unit test if you move this function into a new file MousePositionUtils.js in the utils folder.

then you have to require the function only and use it.

if you need help on how this, do not hesitate to ask

@MV88 MV88 requested a review from offtherailz February 19, 2018 13:29
@offtherailz
Copy link
Member

Don't know why but travis reported your PR as abuse, so the build didn't start and you couldn't notice the eslint issue notified by Matteo.

I tried to add you to collaborators, I hope it helps. Otherwise you may need to contact travis support, because you may be black-listed (or something else, how knows).
Anyway you can check the issues notified by travis running npm run travis to make you sure that everything is ok for the PR.
Thank you for your contribution.

@offtherailz
Copy link
Member

I tried to trigger the build again by closing and opening the pull request but I had the same result.
So I sent a request to support@travis-ci.com to understand the reason of this abuse notification and how to fix it (This may be useful for the future too). We have to wait a reply.

@offtherailz
Copy link
Member

I quote the email from support:

Sorry for the confusion.Our system automatically flagged the Terranex organization for possible abuse, but after further review, we were able to clear that up. Any PRs coming from forks of this organization should be now all set to build as normal, though please feel free to contact us should you have any questions or concerns arise. To trigger a build for this PR, I'd recommend pushing a commit to the branch or just closing and reopening the PR directly from GitHub - Hope this helps!

So @riaanvddool you can commit again and you will see the result from travis build.

@mbarto
Copy link
Contributor

mbarto commented Feb 28, 2018

Hi Riaan, I make your code a bit more generic here: #2648

Going to close this one. Thanks for the contribution

@mbarto mbarto closed this Feb 28, 2018
@ghost ghost removed the In Test label Feb 28, 2018
@riaanvddool
Copy link
Contributor Author

@mbarto Thanks, I will try it out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants