Track map state in URL for easier link sharing #2301
Conversation
ghost
assigned 
Codecov Report
@@ Coverage Diff @@
## master #2301 +/- ##
==========================================
+ Coverage 80.72% 80.79% +0.06%
==========================================
Files 192 192
Lines 12395 12386 -9
Branches 2308 2307 -1
==========================================
+ Hits 10006 10007 +1
+ Misses 1615 1598 -17
- Partials 774 781 +7
Continue to review full report at Codecov.
|
davea
force-pushed
the
2242-automatic-permalink
branch
from
0b5db8a
to
1a32a07
Compare
davea
changed the title
davea
force-pushed
the
2242-automatic-permalink
branch
2 times, most recently
from
4b8ebfc
to
6e1512a
Compare
davea
force-pushed
the
2242-automatic-permalink
branch
from
78678d9
to
755db6c
Compare
|
Thanks for the feedback - I think I've addressed everything! |
There was a problem hiding this comment.
One query about the "problems nearby" exemption logic, is all I think, on the existing code.
The outstanding issue not covered is URLs having lat/lon and latitude/longitude in them. Only use of the long-form is the geocoder disambiguation, so perhaps we should look at switching them to short-form, unless there's some reason why they're not...
| // may have changed since then if e.g. the map filters have been updated. | ||
| // NB this won't change the base of the 'problems nearby' permalink on | ||
| // /report, as this would result in it pointing at the wrong page. | ||
| if (this.base !== '/around' && fixmystreet.page !== 'report') { |
There was a problem hiding this comment.
Should this be or, not and - if you only want to exclude /around on a report page? Don't think it matters for the othe one on a report page, though, as the URL will be the same.
There was a problem hiding this comment.
So, thinking a bit about this, should we even be in this function if we are on a report page? All it's going to do is add cruft to the permalink url.
There was a problem hiding this comment.
You might want to permalink an offset pin? e.g. https://www.fixmystreet.com/report/1234567?zoom=2&lat=51.5588&lon=-1.14649&layers=BT But I agree this seems pretty unlikely.
dracos
force-pushed
the
2242-automatic-permalink
branch
from
99265ec
to
7725eaf
Compare
dracos
force-pushed
the
2242-automatic-permalink
branch
from
6411963
to
c3ae406
Compare
Fixes #2242. Co-authored-by: Matthew Somerville <[email protected]>
Remove the server-side zoom-in-url fix, instead use an ArgParser subclass to default to the provided data if nothing in URL. Then we can switch to using short lat/lon in geocoder URLs.
dracos
force-pushed
the
2242-automatic-permalink
branch
from
c3ae406
to
2ba3a40
Compare
dracos
force-pushed
the
2242-automatic-permalink
branch
from
2ba3a40
to
1ba5f1c
Compare
Fixes #2242.