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.

Sign up for GitHub

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

Make title optional in generated flash component #6065

Merged
merged 4 commits into from
Jan 27, 2025
Merged

Make title optional in generated flash component #6065

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
99b808b
Remove title from generated flash component
SteffenDE Jan 27, 2025
ebe19d6
move error title to text
SteffenDE Jan 27, 2025
d0eea4e
semibold
SteffenDE Jan 27, 2025
1bcb2e1
re-add title, but make it optional
SteffenDE Jan 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 13 additions & 16 deletions installer/templates/phx_web/components/core_components.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ defmodule <%= @web_namespace %>.CoreComponents do
"""
attr :id, :string, doc: "the optional id of flash container"
attr :flash, :map, default: %{}, doc: "the map of flash messages to display"
attr :title, :string, default: nil
attr :kind, :atom, values: [:info, :error], doc: "used for styling and flash lookup"
attr :rest, :global, doc: "the arbitrary HTML attributes to add to the flash container"

Expand All @@ -46,26 +45,26 @@ defmodule <%= @web_namespace %>.CoreComponents do
role="alert"
class={[
"fixed top-2 right-2 mr-2 w-80 sm:w-96 z-50 rounded-lg p-3 ring-1",
@kind == :info && "bg-emerald-50 text-emerald-800 ring-emerald-500 fill-cyan-900",
@kind == :info && "bg-sky-50 text-sky-800 shadow-md ring-sky-500 fill-sky-900",
@kind == :error && "bg-rose-50 text-rose-900 shadow-md ring-rose-500 fill-rose-900"
]}
{@rest}
>
<p :if={@title} class="flex items-center gap-1.5 text-sm font-semibold leading-6">
<.icon :if={@kind == :info} name="hero-information-circle-mini" class="h-4 w-4" />
<.icon :if={@kind == :error} name="hero-exclamation-circle-mini" class="h-4 w-4" />
{@title}
</p>
<p class="mt-2 text-sm leading-5">{msg}</p>
<button type="button" class="group absolute top-1 right-1 p-2" aria-label=<%= maybe_heex_attr_gettext.("close", @gettext) %>>
<.icon name="hero-x-mark-solid" class="h-5 w-5 opacity-40 group-hover:opacity-70" />
</button>
<div class="flex gap-2">
<.icon :if={@kind == :info} name="hero-information-circle-mini" class="h-5 w-5 shrink-0" />
<.icon :if={@kind == :error} name="hero-exclamation-circle-mini" class="h-5 w-5 shrink-0" />
<p class="text-sm leading-5">{msg}</p>
<div class="flex-1" />
<button type="button" class="group flex self-start" aria-label=<%= maybe_heex_attr_gettext.("close", @gettext) %>>
<.icon name="hero-x-mark-solid" class="h-5 w-5 opacity-40 group-hover:opacity-70" />
</button>
</div>
</div>
"""
end

@doc """
Shows the flash group with standard titles and content.
Shows the flash group with standard content.

## Examples

Expand All @@ -77,12 +76,11 @@ defmodule <%= @web_namespace %>.CoreComponents do
def flash_group(assigns) do
~H"""
<div id={@id} aria-live="polite">
<.flash kind={:info} title=<%= maybe_heex_attr_gettext.("Success!", @gettext) %> flash={@flash} />
<.flash kind={:error} title=<%= maybe_heex_attr_gettext.("Error!", @gettext) %> flash={@flash} />
<.flash kind={:info} flash={@flash} />
<.flash kind={:error} flash={@flash} />
<.flash
id="client-error"
kind={:error}
title=<%= maybe_heex_attr_gettext.("We can't find the internet", @gettext) %>
phx-disconnected={show(".phx-client-error #client-error")}
phx-connected={hide("#client-error")}
hidden
Expand All @@ -94,7 +92,6 @@ defmodule <%= @web_namespace %>.CoreComponents do
<.flash
id="server-error"
kind={:error}
title=<%= maybe_heex_attr_gettext.("Something went wrong!", @gettext) %>
phx-disconnected={show(".phx-server-error #server-error")}
phx-connected={hide("#server-error")}
hidden
Expand Down
5 changes: 3 additions & 2 deletions installer/test/phx_new_test.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ defmodule Mix.Tasks.Phx.NewTest do
assert_file("phx_blog/lib/phx_blog_web/components/core_components.ex", fn file ->
assert file =~ "defmodule PhxBlogWeb.CoreComponents"
assert file =~ ~S|aria-label={gettext("close")}|
assert file =~ ~S|<.flash kind={:info} title={gettext("Success!")} flash={@flash} />|
assert file =~ ~S|gettext("Attempting to reconnect")|
end)

assert_file("phx_blog/lib/phx_blog_web/components/layouts.ex", fn file ->
Expand Down Expand Up @@ -567,7 +567,8 @@ defmodule Mix.Tasks.Phx.NewTest do

assert_file("phx_blog/lib/phx_blog_web/components/core_components.ex", fn file ->
assert file =~ ~S|aria-label="close"|
assert file =~ ~S|<.flash kind={:info} title="Success!" flash={@flash} />|
assert file =~ ~S|Attempting to reconnect|
refute file =~ ~S|gettext("Attempting to reconnect")|
end)
end)
end
Expand Down
29 changes: 13 additions & 16 deletions priv/templates/phx.gen.live/core_components.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ defmodule <%= @web_namespace %>.CoreComponents do
"""
attr :id, :string, doc: "the optional id of flash container"
attr :flash, :map, default: %{}, doc: "the map of flash messages to display"
attr :title, :string, default: nil
attr :kind, :atom, values: [:info, :error], doc: "used for styling and flash lookup"
attr :rest, :global, doc: "the arbitrary HTML attributes to add to the flash container"

Expand All @@ -46,26 +45,26 @@ defmodule <%= @web_namespace %>.CoreComponents do
role="alert"
class={[
"fixed top-2 right-2 mr-2 w-80 sm:w-96 z-50 rounded-lg p-3 ring-1",
@kind == :info && "bg-emerald-50 text-emerald-800 ring-emerald-500 fill-cyan-900",
@kind == :info && "bg-sky-50 text-sky-800 shadow-md ring-sky-500 fill-sky-900",
@kind == :error && "bg-rose-50 text-rose-900 shadow-md ring-rose-500 fill-rose-900"
]}
{@rest}
>
<p :if={@title} class="flex items-center gap-1.5 text-sm font-semibold leading-6">
<.icon :if={@kind == :info} name="hero-information-circle-mini" class="h-4 w-4" />
<.icon :if={@kind == :error} name="hero-exclamation-circle-mini" class="h-4 w-4" />
{@title}
</p>
<p class="mt-2 text-sm leading-5">{msg}</p>
<button type="button" class="group absolute top-1 right-1 p-2" aria-label=<%= maybe_heex_attr_gettext.("close", @gettext) %>>
<.icon name="hero-x-mark-solid" class="h-5 w-5 opacity-40 group-hover:opacity-70" />
</button>
<div class="flex gap-2">
<.icon :if={@kind == :info} name="hero-information-circle-mini" class="h-5 w-5 shrink-0" />
<.icon :if={@kind == :error} name="hero-exclamation-circle-mini" class="h-5 w-5 shrink-0" />
<p class="text-sm leading-5">{msg}</p>
<div class="flex-1" />
<button type="button" class="group flex self-start" aria-label=<%= maybe_heex_attr_gettext.("close", @gettext) %>>
<.icon name="hero-x-mark-solid" class="h-5 w-5 opacity-40 group-hover:opacity-70" />
</button>
</div>
</div>
"""
end

@doc """
Shows the flash group with standard titles and content.
Shows the flash group with standard content.

## Examples

Expand All @@ -77,12 +76,11 @@ defmodule <%= @web_namespace %>.CoreComponents do
def flash_group(assigns) do
~H"""
<div id={@id} aria-live="polite">
<.flash kind={:info} title=<%= maybe_heex_attr_gettext.("Success!", @gettext) %> flash={@flash} />
<.flash kind={:error} title=<%= maybe_heex_attr_gettext.("Error!", @gettext) %> flash={@flash} />
<.flash kind={:info} flash={@flash} />
<.flash kind={:error} flash={@flash} />
<.flash
id="client-error"
kind={:error}
title=<%= maybe_heex_attr_gettext.("We can't find the internet", @gettext) %>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

well maybe we do still want the title for those? @josevalim

Copy link
Contributor Author

Choose a reason for hiding this comment

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

or we move the "Something went wrong!" part into the message

phx-disconnected={show(".phx-client-error #client-error")}
phx-connected={hide("#client-error")}
hidden
Expand All @@ -94,7 +92,6 @@ defmodule <%= @web_namespace %>.CoreComponents do
<.flash
id="server-error"
kind={:error}
title=<%= maybe_heex_attr_gettext.("Something went wrong!", @gettext) %>
phx-disconnected={show(".phx-server-error #server-error")}
phx-connected={hide("#server-error")}
hidden
Expand Down
Loading