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

[fms.com] add toggle between aerial and roads maps #3041

Merged
merged 3 commits into from Jul 13, 2020

Conversation

struan
Copy link
Member

@struan struan commented May 21, 2020

Adds a button to the map controls on FMS.com that toggles between the roads and aerial views:

image

aerial

aerial_mobile

[skip changelog]

@codecov
Copy link

codecov bot commented May 21, 2020

Codecov Report

Merging #3041 into master will decrease coverage by 0.00%.
The diff coverage is 41.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3041      +/-   ##
==========================================
- Coverage   83.27%   83.26%   -0.01%     
==========================================
  Files         248      248              
  Lines       15561    15564       +3     
  Branches     2905     2906       +1     
==========================================
+ Hits        12959    12960       +1     
  Misses       1677     1677              
- Partials      925      927       +2     
Impacted Files Coverage Δ
perllib/FixMyStreet/Map/Bromley.pm 66.66% <0.00%> (ø)
perllib/FixMyStreet/Map/Bing.pm 31.81% <40.00%> (+5.50%) ⬆️
perllib/FixMyStreet/Map/FMS.pm 72.22% <50.00%> (ø)
perllib/Utils.pm 97.97% <0.00%> (-1.02%) ⬇️

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 f55c261...009c424. Read the comment docs.

@struan
Copy link
Member Author

struan commented Jun 9, 2020

@davea davea force-pushed the issues/commercial/1870-aerial-maps branch 2 times, most recently from 67e9c68 to defbbe1 Compare July 1, 2020 14:36
Copy link
Member

@davea davea left a comment

Choose a reason for hiding this comment

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

LGTM. @dracos would you mind checking the non-JS commit please?

@davea davea marked this pull request as ready for review July 1, 2020 14:45
@dracos dracos force-pushed the issues/commercial/1870-aerial-maps branch from defbbe1 to d23452c Compare July 9, 2020 17:14
@dracos
Copy link
Member

dracos commented Jul 9, 2020

Reviewed; then made some changes to bring the Bing URLs up to date, add attribution for the aerial layer, move it all to the base Bing so it can have this as well, and got it so the toggle works on report pages, and so MasterMap-based maps work. @struan Could you review my commits?

Copy link
Member Author

@struan struan left a comment

Choose a reason for hiding this comment

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

One small comment. I would approve but I can only comment as it's my PR :(

[% ELSE %]
<a class="" id="map_layer_toggle" href="[% c.uri_with( { aerial => 1 } ) %]">[% loc('Aerial') %]</a>
<a id="map_layer_toggle" href="[% c.uri_with( { aerial => 1 } ) %]">[% loc('Aerial') %]</a>
Copy link
Member Author

Choose a reason for hiding this comment

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

One small thing is that the link colours for this on e.g. Peterborough end up being dark blue on black so I suspect some CSS munging needs to happen.

dracos and others added 3 commits July 10, 2020 19:09
Co-authored-by: Dave Arter <davea@mysociety.org>
Co-authored-by: Matthew Somerville <matthew@mysociety.org>
@dracos dracos force-pushed the issues/commercial/1870-aerial-maps branch from 2040b84 to 009c424 Compare July 10, 2020 19:05
@dracos dracos merged commit 3d2b2ef into master Jul 13, 2020
@dracos dracos mentioned this pull request Jul 14, 2020
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