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

A few suggested changes, typos, clarrifications etc #95

Open
kitmacleod opened this issue May 13, 2018 · 0 comments
Open

A few suggested changes, typos, clarrifications etc #95

kitmacleod opened this issue May 13, 2018 · 0 comments

Comments

@kitmacleod
Copy link

First, thanks for this important resource for learning OL. Second, I appreciate this resource is being refactored e.g. move to OL5, JS named imports etc. Third, I also appreciate multiple suggestions per issue/pull request are not best GH practice. Since Tim et al you are revising this repo, I wanted to share these suggestions from going through the workshop (it is great that in v5 you have improved the webpack config e.g. removed the webpack-dashboard dependence). Caveat: I am not an expert JS dev.

Suggested changes (SC), potential errors (pe), typos, clarifications and questions :
SC-provide links to main tools that are used to build this. It is great you are using webpack, and a link to the excellent webpack pages would help a reader use this valuable learning resource.
http://openlayers.org/workshop/en/basics/map.html
SC-already have an index.html and main.js in the root, so maybe refer to these.
http://openlayers.org/workshop/en/basics/geolocation.html
Typo- repeated ‘on’ first paragraph
http://openlayers.org/workshop/en/basics/point-feature.html
SC- create link to working code for each ‘when done, the result should look like this;’, the code is in the ‘examples’ folder.
http://openlayers.org/workshop/en/basics/popup.html
SC- replace var overlay with const (or let) overlay; used const elsewhere.
http://openlayers.org/workshop/en/vector/draw.html
SC- the point made is not clear (what is enum, why do this) “Note that we could have also imported the GeometryType enum (import GeometryType from 'ol/geom/geometrytype';) and used GeometryType.POLYGON instead of the 'Polygon' string above.”
http://openlayers.org/workshop/en/vector/style.html
PE- if ‘Static style: if we wanted to give all features…’ code is added then an error is returned
ReferenceError: Style is not defined at Object.defineProperty.value
SC- “Next we’ll write a couple of functions to determine a color based on area of a geometry’, need to provide more information on these two functions esp. what does ‘function clamp’ do.
Clarify- “And now we can add a function that creates a style with a fill color…” Is this a new function appended to existing code or modifying earlier ‘const layer = new VectorLayer({‘ ? It is the former, as when script started get an error “duplicate declaration ‘layer’”
SC- To render map, may need to say that countries.GeoJSON file needs to be added again.
NextSection: Vector Tiles
http://openlayers.org/workshop/en/vectortile/bright.html
SC- with all the ‘cleaning up’ (or refactoring) need to provide a link/text on revised code.
Q- what about the html style code (remove as well? If so, say explicitly).
NextLesson Raster Operations
http://openlayers.org/workshop/en/raster/
Check “You should now see some oddly colored tiles shown over your base layer.” Link is not showing the RGB tiles.
Typo “Add this layer (to) your map's layers array in..>”
Unable to show elevation /flood data in my example or their linked map.
SC- may be missing https://openlayers.org/en/latest/apidoc/ol.source.Raster.html
SC- Need to use this .js as main.js https://github.com/openlayers/workshop/blob/master/src/en/examples/raster/raster.js

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

No branches or pull requests

1 participant