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

Adds geojson of building g (campus III) #213

Merged
merged 6 commits into from
Feb 9, 2022

Conversation

oleschl
Copy link
Contributor

@oleschl oleschl commented Feb 1, 2022

Indoor map for building / campus III. Closes the second task of #190.

@oleschl oleschl self-assigned this Feb 1, 2022
@ClFeSc
Copy link
Contributor

ClFeSc commented Feb 2, 2022

As you have assigned us as reviewers, can we assume that this PR is ready and you are expecting no more changes? The related issue is still in Develop (Ongoing).

@laugengebaeck
Copy link
Contributor

laugengebaeck commented Feb 7, 2022

Just a quick heads-up from team TOBIAS: In #227, we made some changes to the GeoJSON code to also use it for the buildings.

  • There is now a separate folder geojsons inside of assets, because we felt mixing images and GeoJSONs in one folder would become confusing very quickly.
  • The GeoJSON filenames for the indoor maps are now declared in the list geojsonLayerFiles in leafletMap.js.

Could you maybe adapt your code to these small changes? Thank you! ^^

@codecov
Copy link

codecov bot commented Feb 7, 2022

Codecov Report

Merging #213 (e4f6b6b) into dev (8bee1f7) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##              dev     #213   +/-   ##
=======================================
  Coverage   98.72%   98.72%           
=======================================
  Files          26       26           
  Lines         470      470           
=======================================
  Hits          464      464           
  Misses          6        6           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8bee1f7...e4f6b6b. Read the comment docs.

@JulianEgbert
Copy link
Contributor

Just a quick heads-up from team TF: In #227, we made some changes to the GeoJSON code to also use it for the buildings.

  • There is now a separate folder geojsons inside of assets, because we felt mixing images and GeoJSONs in one folder would become confusing very quickly.
  • The GeoJSON filenames for the indoor maps are now declared in the list geojsonLayerFiles in leafletMap.js.

Could you maybe adapt your code to these small changes? Thank you! ^^

I just did this and made some fixes with the naming of the files.
Now they are consistent with each other.

@oleschl
Copy link
Contributor Author

oleschl commented Feb 8, 2022

As you have assigned us as reviewers, can we assume that this PR is ready and you are expecting no more changes? The related issue is still in Develop (Ongoing).

It is ready to be merged. I just have not been sure because the issue is about campus II + III. However we decided to split it.

spec/features/building_map_spec.rb Outdated Show resolved Hide resolved
spec/features/building_map_spec.rb Outdated Show resolved Hide resolved
@oleschl oleschl merged commit f56ec32 into dev Feb 9, 2022
@oleschl oleschl deleted the feature/hg_190_indoor-map-campus-III branch February 9, 2022 09:39
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.

4 participants