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

CRS Selector Merge #3798

Merged
merged 52 commits into from
May 31, 2019
Merged

CRS Selector Merge #3798

merged 52 commits into from
May 31, 2019

Conversation

tdipisa
Copy link
Member

@tdipisa tdipisa commented May 24, 2019

Description

This PR includes the new CRS selector feature into master

Issues

URL of RTD document: https://mapstore.readthedocs.io/en/c042_crs/ Documentation Status

@rtd-helper
Copy link

rtd-helper bot commented May 24, 2019

WebSocket is not open: readyState 3 (CLOSED)

@offtherailz offtherailz requested a review from mbarto May 29, 2019 12:59
@coveralls
Copy link

coveralls commented May 29, 2019

Coverage Status

Coverage increased (+0.02%) to 81.421% when pulling 91a9e81 on c042_crs into 220db17 on 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.

Did you create new files for glyph icons (eot, ttf, etc.) with new icons from master and new icons from this work? Binary merge will not work, so we are going to loose icons if this is not properly done.

@@ -0,0 +1,12 @@
var expect = require('expect');
Copy link
Contributor

Choose a reason for hiding this comment

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

const would be better, instead of var, our style guide rule for this is to avoid using var

describe('Test crsselector actions', () => {
it('test input value action', () => {
let value = 'ESPG:4326';
const retVal = setInputValue(value);
Copy link
Contributor

Choose a reason for hiding this comment

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

I would remove value, and directly use 'EPSG:4326' as the setInputValue argument

@@ -30,6 +30,7 @@ class LeafletMap extends React.Component {
onClick: PropTypes.func,
onRightClick: PropTypes.func,
mapOptions: PropTypes.object,
limits: PropTypes.limits,
Copy link
Contributor

Choose a reason for hiding this comment

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

PropTypes.object, I presume

@@ -141,9 +141,13 @@ Layers.registerType('wms', {
urls: urls,
params: queryParameters,
tileGrid: new ol.tilegrid.TileGrid({
// TODO: custom grid sets with custom extent
Copy link
Contributor

Choose a reason for hiding this comment

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

only TODO comments, I don't think this is useful

@offtherailz
Copy link
Member

@mbarto I made @allyoucanmap generate a file with icons from master and this new yesterday. I hope it's enough.

@mbarto
Copy link
Contributor

mbarto commented May 29, 2019

@offtherailz yes, if you have generated yesterday should be fine

@tdipisa tdipisa removed the Blocked label May 31, 2019
@offtherailz offtherailz merged commit 6e94af4 into master May 31, 2019
@offtherailz offtherailz deleted the c042_crs branch June 28, 2022 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants