[openstreetmap/openstreetmap-website] Use Turbo frames for sidebar loading (PR #7245)

[Tom Hughes]

@tomhughes commented on this pull request.



> @@ -208,7 +208,7 @@ def set_locale
 
     I18n.locale = Locale.available.preferred(preferred_languages)
 
-    response.headers["Vary"] = "Accept-Language"
+    response.headers["Vary"] = "Accept-Language, Turbo-Frame"

I don't think a method called `set_locale` is the right place to be doing this, even if it is convenient.

I think either we need a separate action filter for this, or possibly we need to rename this to something more generic - are we sure that all possible requests that might use turbo go through here? I guess they probably should since this is needed to setup the locale correctly?

> @@ -263,7 +263,7 @@ def map_layout
 
     flash.now[:warning] = { :partial => "layouts/offline_flash" } unless api_status == "online"
 
-    request.xhr? ? "xhr" : "map"
+    request.headers["turbo-frame"] == "sidebar_content_frame" ? "xhr" : "map"

I believe you can use `turbo_frame_request?` here instead of having to know the internal implementation details:

```suggestion
    turbo_frame_request? ? "xhr" : "map"
```

or if it's important which frame it is then:

```suggestion
    turbo_frame_request_id == "sidebar_content_frame" ? "xhr" : "map"
```

> @@ -263,7 +263,7 @@ def map_layout
 
     flash.now[:warning] = { :partial => "layouts/offline_flash" } unless api_status == "online"
 
-    request.xhr? ? "xhr" : "map"
+    request.headers["turbo-frame"] == "sidebar_content_frame" ? "xhr" : "map"

Turbo already overrides the layout to use `turbo_rails/frame` by default and you can provide your own version of that if you want though I guess that doesn't work if we only want this for some requests.

Is it really correct to put the `turbo-frame` wrapper in the layout though or should we be putting it in each place that renders sidebar content?

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/7245#pullrequestreview-4747583062
You are receiving this because you are subscribed to this thread.

Message ID: <openstreetmap/openstreetmap-website/pull/7245/review/4747583062 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20260721/1da21c75/attachment-0001.htm>