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

Add opt-in modal behaviour to dialogs #5661

Closed
wants to merge 2 commits into from
Closed

Add opt-in modal behaviour to dialogs #5661

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cda69ea
Add attribute 'modal' to prevent accidental closing
ArthurClemens Dec 9, 2023
d5eeb11
Add modal behaviour to form dialogs
ArthurClemens Dec 9, 2023
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
4 changes: 3 additions & 1 deletion 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 @@ -35,9 +35,11 @@ defmodule <%= @web_namespace %>.CoreComponents do
This is another modal.
</.modal>

To prevent the modal from accidentally closing when clicking on the backdrop, pass the 'modal' attr.
"""
attr :id, :string, required: true
attr :show, :boolean, default: false
attr :modal, :boolean, default: false
Copy link
Contributor

Choose a reason for hiding this comment

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

I like the idea of conditionally disabling the phx-click-away, but I feel the name of the attribute is a bit strange since the component is already called "modal".

Perhaps this was something you struggled with as well?

Brainstorming a bit:

  • persistent
  • sticky
  • noBackdropDismiss
  • preventBackdropClose

Copy link
Author

Choose a reason for hiding this comment

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

Well, I wouldn't have called it a modal of course :-)

Perhaps blocking has a similar ring.

Copy link

Choose a reason for hiding this comment

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

confirmClose maybe?

Copy link
Author

Choose a reason for hiding this comment

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

These are separate concerns. Modal behaviour means that interacting with the main content is disabled until the user interacts with the dialog content. That means that clicking on the backdrop does nothing, not even asking for a confirmation.
This type of interaction is common for confirmation dialogs: they're not meant to be closed without clicking OK or Cancel.
In #5660 we agreed that asking for a confirmation provides a better UX for forms in a dialog. Nevertheless there will still be a need to be able to create actual modal dialogs.

Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this also apply to pressing esc?

Copy link
Author

@ArthurClemens ArthurClemens Dec 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

For accessibility, Escape to close should always be possible.
See the W3C Modal Dialog Example

Copy link
Member

Choose a reason for hiding this comment

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

That's the one that trips me the most. So perhaps the best solution is to ask for confirmation indeed. Let's see what Chris has to say after the holidays. :)

attr :on_cancel, JS, default: %JS{}
slot :inner_block, required: true

Expand Down Expand Up @@ -65,7 +67,7 @@ defmodule <%= @web_namespace %>.CoreComponents do
id={"#{@id}-container"}
phx-window-keydown={JS.exec("data-cancel", to: "##{@id}")}
phx-key="escape"
phx-click-away={JS.exec("data-cancel", to: "##{@id}")}
phx-click-away={not @modal and JS.exec("data-cancel", to: "##{@id}")}
class="shadow-zinc-700/10 ring-zinc-700/10 relative hidden rounded-2xl bg-white p-14 shadow-lg ring-1 transition"
>
<div class="absolute top-6 right-5">
Expand Down
4 changes: 3 additions & 1 deletion 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 @@ -35,9 +35,11 @@ defmodule <%= @web_namespace %>.CoreComponents do
This is another modal.
</.modal>

To prevent the modal from accidentally closing when clicking on the backdrop, pass the 'modal' attr.
"""
attr :id, :string, required: true
attr :show, :boolean, default: false
attr :modal, :boolean, default: false
attr :on_cancel, JS, default: %JS{}
slot :inner_block, required: true

Expand Down Expand Up @@ -65,7 +67,7 @@ defmodule <%= @web_namespace %>.CoreComponents do
id={"#{@id}-container"}
phx-window-keydown={JS.exec("data-cancel", to: "##{@id}")}
phx-key="escape"
phx-click-away={JS.exec("data-cancel", to: "##{@id}")}
phx-click-away={not @modal and JS.exec("data-cancel", to: "##{@id}")}
class="shadow-zinc-700/10 ring-zinc-700/10 relative hidden rounded-2xl bg-white p-14 shadow-lg ring-1 transition"
>
<div class="absolute top-6 right-5">
Expand Down
2 changes: 1 addition & 1 deletion priv/templates/phx.gen.live/index.html.heex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
</:action>
</.table>

<.modal :if={@live_action in [:new, :edit]} id="<%= schema.singular %>-modal" show on_cancel={JS.patch(~p"<%= schema.route_prefix %>")}>
<.modal :if={@live_action in [:new, :edit]} id="<%= schema.singular %>-modal" show modal on_cancel={JS.patch(~p"<%= schema.route_prefix %>")}>
<.live_component
module={<%= inspect context.web_module %>.<%= inspect Module.concat(schema.web_namespace, schema.alias) %>Live.FormComponent}
id={@<%= schema.singular %>.id || :new}
Expand Down
2 changes: 1 addition & 1 deletion priv/templates/phx.gen.live/show.html.heex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

<.back navigate={~p"<%= schema.route_prefix %>"}>Back to <%= schema.plural %></.back>

<.modal :if={@live_action == :edit} id="<%= schema.singular %>-modal" show on_cancel={JS.patch(~p"<%= schema.route_prefix %>/#{@<%= schema.singular %>}")}>
<.modal :if={@live_action == :edit} id="<%= schema.singular %>-modal" show modal on_cancel={JS.patch(~p"<%= schema.route_prefix %>/#{@<%= schema.singular %>}")}>
<.live_component
module={<%= inspect context.web_module %>.<%= inspect Module.concat(schema.web_namespace, schema.alias) %>Live.FormComponent}
id={@<%= schema.singular %>.id}
Expand Down