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

Update openlayers to v6.5.0 #651

Merged
merged 13 commits into from
May 28, 2021
Merged

Conversation

simonseyock
Copy link
Member

This are the first steps to update openlayers to v6.4.3

@simonseyock
Copy link
Member Author

I continued to work on this.

  • For the dependency management I added a link to the tagged version on github for openlayers. This is done so the openlayers build-legacy script can be called and a legacy style .js file can be included (so we can use for example new ol.Map). The releases from openlayers.org do not include legacy builds anymore. Probably all projects that would like to use geoext with openlayers v6 would need to do that as well if we stick with this. I did not look into what needs to be done to use the newer import syntax (i.e. import Map from 'ol/Map';), but I suppose this would be lots and lots of repetetive rewriting - even with clever use of find and replace.
  • I rewrote the examples to directly include this openlayers version.
  • Since openlayers v6.0.0 maps cannot share layers anymore. I removed the ability to use the layers of the parent map in the overlay map as there is no sensible way to clone a layer.

@simonseyock simonseyock changed the title WIP: Update openlayers to v6.4.3 WIP: Update openlayers to v6.5.0 Mar 26, 2021
@simonseyock
Copy link
Member Author

It might also be possible to either host a built legacy ol version somewhere, publish it on npm or put it in the repository.

@simonseyock
Copy link
Member Author

examples/features/grid-spatial-filter.html does not seem to work because of CORS and/or the remote server. I am not sure about that.

@simonseyock
Copy link
Member Author

examples/print/basic-mapfish.html is also broken, but this seems not to be an openlayers issue, but that in GeoExt.data.serializer.Vector canvas.drawImage is called with an image of size [0, 0]

@simonseyock
Copy link
Member Author

All other examples seem to work. So feel free to test this, but mind my comment about the legacy build if you want to include this in another project. If you just checkout this branch, you should just need to call npm install and should be ready to try out the examples.

What do you think about the legacy build issue @marcjansen ?

@simonseyock simonseyock changed the title WIP: Update openlayers to v6.5.0 Update openlayers to v6.5.0 Mar 26, 2021
@simonseyock
Copy link
Member Author

And I ported the cascadeLayers function from BasiGX

@enorton
Copy link

enorton commented Mar 26, 2021

Just here to cheer you on. :D

@marcjansen
Copy link
Member

Great to see some progress here, thanks @simonseyock!

Wrt the next steps we should have a discussion also with other devs, I guess.

@marcjansen
Copy link
Member

I requested some reviews, but think this is not meant to be merged right now.

package.json Outdated Show resolved Hide resolved
@simonseyock
Copy link
Member Author

simonseyock commented Apr 16, 2021

Roadmap:

  1. Release a geoext version (v3.4.0) with the current master - create backlog branch
  2. Rename repository to geoext (rename current geoext to geoext1, maybe archive geoext1 and geoext2)
  3. Publish openlayers legacy package on npm (maybe scoped to @geoext) See https://www.npmjs.com/package/@geoext/openlayers-legacy
  4. Publish new geoext version (v4.0.0) with Migration Notes (overview map, scoped listeners)
  5. Release a new BasiGX version (v2.3.0) - create backlog branch
  6. Upgrade openlayers and geoext in BasiGX
  7. Publish new BasiGX version (v3.0.0) with Migration Notes
  8. Eventually upgrade a custom project

@marcjansen
Copy link
Member

Awesome, sounds great!

@coveralls
Copy link

coveralls commented May 27, 2021

Pull Request Test Coverage Report for Build 885110934

  • 54 of 65 (83.08%) changed or added relevant lines in 9 files are covered.
  • 4 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.02%) to 81.382%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/component/Popup.js 2 3 66.67%
src/data/model/OlObject.js 2 3 66.67%
src/data/store/Layers.js 11 12 91.67%
src/component/OverviewMap.js 9 12 75.0%
src/util/Layer.js 5 10 50.0%
Files with Coverage Reduction New Missed Lines %
src/component/OverviewMap.js 2 76.89%
src/data/MapfishPrintProvider.js 2 83.19%
Totals Coverage Status
Change from base Build 884853549: -0.02%
Covered Lines: 1402
Relevant Lines: 1646

💛 - Coveralls

@simonseyock simonseyock force-pushed the update-ol branch 2 times, most recently from ef8536d to f2feab4 Compare May 28, 2021 08:44
@marcjansen
Copy link
Member

Can you verify that the SymbolCheck mixin still works as expected?

@simonseyock
Copy link
Member Author

SymbolCheck works as expected. It could be argued if it should get removed in the future as geoext now depends on a fixed openlayers version which should contain all expected symbols. But as @marcjansen mentioned it does not produce any overhead so it makes sense to keep it for now.

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

6 participants