From 44933acebb071bdf3e01765938f12b39310176ce Mon Sep 17 00:00:00 2001 From: jamey Date: Fri, 13 Feb 2026 16:21:51 +0000 Subject: [PATCH] fix search modal race condition and add 304 support for images Route all search modal open/close through the JS hook via custom DOM events so the _closing flag is always correctly managed. Prevents the modal flashing back after Escape when a search response is in flight. Add If-None-Match / 304 Not Modified handling to the image controller so browsers don't re-download images on revalidation. Co-Authored-By: Claude Opus 4.6 --- assets/js/app.js | 6 + .../components/shop_components/layout.ex | 18 +-- .../controllers/image_controller.ex | 148 +++++++++++------- 3 files changed, 98 insertions(+), 74 deletions(-) diff --git a/assets/js/app.js b/assets/js/app.js index 1b0a167..0bcfb70 100644 --- a/assets/js/app.js +++ b/assets/js/app.js @@ -341,6 +341,9 @@ const SearchModal = { } document.addEventListener("keydown", this._globalKeydown) + this.el.addEventListener("open-search", () => this.open()) + this.el.addEventListener("close-search", () => this.close()) + this.el.addEventListener("keydown", (e) => { if (!this.isOpen()) return @@ -370,6 +373,7 @@ const SearchModal = { }, updated() { + if (this._closing) this.el.style.display = "none" this.selectedIndex = -1 this.updateHighlight() }, @@ -379,6 +383,7 @@ const SearchModal = { }, open() { + this._closing = false this.el.style.display = "flex" this.pushEvent("open_search", {}) const input = this.el.querySelector("#search-input") @@ -391,6 +396,7 @@ const SearchModal = { }, close() { + this._closing = true this.el.style.display = "none" this.pushEvent("clear_search", {}) this.selectedIndex = -1 diff --git a/lib/simpleshop_theme_web/components/shop_components/layout.ex b/lib/simpleshop_theme_web/components/shop_components/layout.ex index ac3f7a4..501d82f 100644 --- a/lib/simpleshop_theme_web/components/shop_components/layout.ex +++ b/lib/simpleshop_theme_web/components/shop_components/layout.ex @@ -383,10 +383,7 @@ defmodule SimpleshopThemeWeb.ShopComponents.Layout do class="search-modal" style={"position: fixed; inset: 0; background: rgba(0,0,0,0.5); z-index: 1001; display: #{if @search_open, do: "flex", else: "none"}; align-items: flex-start; justify-content: center; padding-top: 10vh;"} phx-hook="SearchModal" - phx-click={ - Phoenix.LiveView.JS.hide(to: "#search-modal") - |> Phoenix.LiveView.JS.push("clear_search") - } + phx-click={Phoenix.LiveView.JS.dispatch("close-search", to: "#search-modal")} >
Phoenix.LiveView.JS.push("clear_search") - } + phx-click={Phoenix.LiveView.JS.dispatch("close-search", to: "#search-modal")} aria-label="Close search" >
Phoenix.LiveView.JS.show(to: "#search-modal", display: "flex") - |> Phoenix.LiveView.JS.focus(to: "#search-input") - } + phx-click={Phoenix.LiveView.JS.dispatch("open-search", to: "#search-modal")} aria-label="Search" > id}) do - case Media.get_image(id) do - nil -> - send_resp(conn, 404, "Image not found") + etag = ~s("#{id}") - image -> - conn - |> put_resp_content_type(image.content_type) - |> put_resp_header("cache-control", "public, max-age=31536000, immutable") - |> put_resp_header("etag", ~s("#{image.id}")) - |> send_resp(200, image.data) + if etag_match?(conn, etag) do + send_not_modified(conn, etag) + else + case Media.get_image(id) do + nil -> + send_resp(conn, 404, "Image not found") + + image -> + conn + |> put_resp_content_type(image.content_type) + |> put_resp_header("cache-control", "public, max-age=31536000, immutable") + |> put_resp_header("etag", etag) + |> send_resp(200, image.data) + end end end @@ -32,40 +38,43 @@ defmodule SimpleshopThemeWeb.ImageController do it on-demand and saves it for future requests. """ def thumbnail(conn, %{"id" => id}) do - thumb_path = Media.get_thumbnail_path(id) + etag = ~s("#{id}-thumb") - if File.exists?(thumb_path) do - # Serve from disk cache - conn - |> put_resp_content_type("image/jpeg") - |> put_resp_header("cache-control", "public, max-age=31536000, immutable") - |> put_resp_header("etag", ~s("#{id}-thumb")) - |> send_file(200, thumb_path) + if etag_match?(conn, etag) do + send_not_modified(conn, etag) else - # Thumbnail not yet generated - generate on-demand - case Media.get_image(id) do - nil -> - send_resp(conn, 404, "Image not found") + thumb_path = Media.get_thumbnail_path(id) - %{data: data} when is_binary(data) -> - case generate_thumbnail_on_demand(data, thumb_path) do - {:ok, binary} -> - conn - |> put_resp_content_type("image/jpeg") - |> put_resp_header("cache-control", "public, max-age=31536000, immutable") - |> put_resp_header("etag", ~s("#{id}-thumb")) - |> send_resp(200, binary) + if File.exists?(thumb_path) do + conn + |> put_resp_content_type("image/jpeg") + |> put_resp_header("cache-control", "public, max-age=31536000, immutable") + |> put_resp_header("etag", etag) + |> send_file(200, thumb_path) + else + case Media.get_image(id) do + nil -> + send_resp(conn, 404, "Image not found") - {:error, _} -> - # Fallback to full image if thumbnail generation fails - conn - |> put_resp_content_type("image/webp") - |> put_resp_header("cache-control", "public, max-age=31536000, immutable") - |> send_resp(200, data) - end + %{data: data} when is_binary(data) -> + case generate_thumbnail_on_demand(data, thumb_path) do + {:ok, binary} -> + conn + |> put_resp_content_type("image/jpeg") + |> put_resp_header("cache-control", "public, max-age=31536000, immutable") + |> put_resp_header("etag", etag) + |> send_resp(200, binary) - %{data: nil} -> - send_resp(conn, 404, "Image data not available") + {:error, _} -> + conn + |> put_resp_content_type("image/webp") + |> put_resp_header("cache-control", "public, max-age=31536000, immutable") + |> send_resp(200, data) + end + + %{data: nil} -> + send_resp(conn, 404, "Image data not available") + end end end end @@ -107,31 +116,36 @@ defmodule SimpleshopThemeWeb.ImageController do alias SimpleshopTheme.Images.Optimizer with {width, format} <- parse_width_and_format(width_with_ext), - true <- width in Optimizer.all_widths(), - %{data: data} when is_binary(data) <- Media.get_image(id) do - case Optimizer.generate_variant_on_demand(data, id, width, format) do - {:ok, path} -> - conn - |> put_resp_content_type(format_content_type(format)) - |> put_resp_header("cache-control", "public, max-age=31536000, immutable") - |> put_resp_header("etag", ~s("#{id}-#{width}.#{format}")) - |> send_file(200, path) + true <- width in Optimizer.all_widths() do + etag = ~s("#{id}-#{width}.#{format}") - {:error, _reason} -> - send_resp(conn, 500, "Failed to generate variant") + if etag_match?(conn, etag) do + send_not_modified(conn, etag) + else + case Media.get_image(id) do + %{data: data} when is_binary(data) -> + case Optimizer.generate_variant_on_demand(data, id, width, format) do + {:ok, path} -> + conn + |> put_resp_content_type(format_content_type(format)) + |> put_resp_header("cache-control", "public, max-age=31536000, immutable") + |> put_resp_header("etag", etag) + |> send_file(200, path) + + {:error, _reason} -> + send_resp(conn, 500, "Failed to generate variant") + end + + nil -> + send_resp(conn, 404, "Image not found") + + %{data: nil} -> + send_resp(conn, 404, "Image data not available") + end end else - :error -> - send_resp(conn, 400, "Invalid width or format") - - false -> - send_resp(conn, 400, "Width not supported") - - nil -> - send_resp(conn, 404, "Image not found") - - %{data: nil} -> - send_resp(conn, 404, "Image data not available") + :error -> send_resp(conn, 400, "Invalid width or format") + false -> send_resp(conn, 400, "Width not supported") end end @@ -194,4 +208,18 @@ defmodule SimpleshopThemeWeb.ImageController do "#" <> color end end + + defp etag_match?(conn, etag) do + case Plug.Conn.get_req_header(conn, "if-none-match") do + [^etag] -> true + _ -> false + end + end + + defp send_not_modified(conn, etag) do + conn + |> put_resp_header("cache-control", "public, max-age=31536000, immutable") + |> put_resp_header("etag", etag) + |> send_resp(304, "") + end end