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

Allow overwriting of map_tooltip and map_crosshair #2835

Merged
merged 5 commits into from Aug 28, 2017

Conversation

pfirpfel
Copy link
Contributor

No description provided.

@pfirpfel pfirpfel requested a review from fredj August 24, 2017 13:22
/**
* Sets the map crosshair to the center (or the map center if nothing provided).
* Overwrites an existing map crosshair.
* @param {?ol.Coordinate|undefined} center Optional center coordinate.
Copy link
Member

Choose a reason for hiding this comment

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

Instead of having {?ol.Coordinate|undefined} use {?ol.Coordinate=} an rename the parameter to opt_center.
This is the standard way for optional parameter with the closure compiler.

* Sets the map tooltip to the center (or the map center if nothing provided).
* Overwrites an existing map tooltip.
* @param {string} tooltipText Text to display in tooltip.
* @param {?ol.Coordinate|undefined} center Optional center coordinate.
Copy link
Member

Choose a reason for hiding this comment

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

Same remark as above

* Sets the map crosshair to the center (or the map center if nothing provided).
* Overwrites an existing map crosshair.
* @param {?ol.Coordinate|undefined} center Optional center coordinate.
* @export
Copy link
Member

Choose a reason for hiding this comment

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

If this function is not used in a html template, the @export is not needed

Copy link
Member

@fredj fredj left a comment

Choose a reason for hiding this comment

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

Looks very good ! Thanks

@pfirpfel pfirpfel self-assigned this Aug 24, 2017
@fredj
Copy link
Member

fredj commented Aug 28, 2017

please merge if you think it's ready

@pfirpfel pfirpfel merged commit 12b1dbe into camptocamp:master Aug 28, 2017
@pfirpfel pfirpfel deleted the feature/permalink-setters branch October 30, 2017 08:32
pfirpfel added a commit to pfirpfel/ngeo that referenced this pull request Oct 30, 2017
* Add setter for map crosshair

* Add setter for tooltip
pfirpfel added a commit that referenced this pull request Nov 2, 2017
* Add setter for map crosshair

* Add setter for tooltip
@sbrunner sbrunner added this to the 2.3 milestone Apr 12, 2018
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.

None yet

3 participants