@hlfan commented on this pull request.
>
- if (id.data("gpx")) {
- params.set("gpx", id.data("gpx"));
- } else if (hashParams.gpx) {
- params.set("gpx", hashParams.gpx);
- }
+ params.set("map", [
+ (mapParams.object && hashArgs.center && hashArgs.zoom) || (id.data("lat")
&& id.data("lon") && 16) || mapParams.zoom || 17,
+ (mapParams.object && hashArgs.center?.lat) || (id.data("lon") &&
id.data("lat")) || mapParams.lat,
+ (mapParams.object && hashArgs.center?.lng) || (id.data("lat") &&
id.data("lon")) || mapParams.lon
+ ].join("/"));
It decouples the parameters' conditions, opening them up to simplification.
``` js
[
mapParams.zoom || 17, // dropping hashArgs, this is already used in
mapParams if it exists,
// and the secondary default value of 16
hashArgs.center?.lat || id.data("lat") || mapParams.lat,
hashArgs.center?.lng || id.data("lon") || mapParams.lon
]
```
And I'd argue this is less hard to reason about than a version written with
`if`-`else if`.
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5597#discussion_r1940541052
You are receiving this because you are subscribed to this thread.
Message ID:
<openstreetmap/openstreetmap-website/pull/5597/review/2591795...@github.com>_______________________________________________
rails-dev mailing list
rails-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/rails-dev
