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

Add components for floating map logo and scale combo #15

Merged
merged 9 commits into from
Sep 6, 2017

Conversation

ahennr
Copy link
Member

@ahennr ahennr commented Sep 6, 2017

Introduces FloatingMapLogo and ScaleCombo components.

In addition, MapUtil is included here as well beside method buildLayersFromInitialState (see #12)

* @param {type} resolution Description
* @param {type} mv Description
*
* @return {type} Description
Copy link
Member

Choose a reason for hiding this comment

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

Wrong JSDoc.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in bef0bbd

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in bef0bbd

* @param {type} resolution Description
* @param {type} mv Description
*
* @return {type} Description
Copy link
Member

Choose a reason for hiding this comment

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

Wrong JSDoc.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in bef0bbd

import FloatingMapLogo from './FloatingMapLogo.jsx'; //@react-geo@

render(
<FloatingMapLogo imageSrc="logo.png"/>,
Copy link
Member

Choose a reason for hiding this comment

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

Can we make use of a image-loader here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Alright, use image loader in 0607940

let scale = MapUtils.getScaleForResolution(resolution, mv.getProjection().getUnits());
let roundScale = Math.round(scale);
let option = <Option key={roundScale.toString()} value={roundScale.toString()}>1:{roundScale.toLocaleString()}</Option>;
this.props.scales.push(option);
Copy link
Member

Choose a reason for hiding this comment

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

Props are immutable inside the component. Please make use of the state instead.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in bef0bbd

*
* @return {Element} Option element for provided zoom level
*/
getVal (zoom) {
Copy link
Member

Choose a reason for hiding this comment

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

Can we rename this to something more meaningful?

Copy link
Member Author

Choose a reason for hiding this comment

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

Renamed to determineOptionKeyForZoomLevel in bef0bbd

@dnlkoch
Copy link
Member

dnlkoch commented Sep 6, 2017

Top! Great addition!

@ahennr
Copy link
Member Author

ahennr commented Sep 6, 2017

Thx for review @dnlkoch . Will merge as soon as all checks are green.

@ahennr ahennr merged commit cc9654b into terrestris:master Sep 6, 2017
@ahennr ahennr deleted the add-maplogo-and-scale-combo branch September 6, 2017 15:45
@KaiVolland KaiVolland mentioned this pull request Nov 29, 2017
56 tasks
hblitza pushed a commit that referenced this pull request Oct 19, 2022
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.

2 participants