Add item actions to show view #1235
Open
+161
−62
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In the example with soft delete of users, it would be best if the user handles the case to redirect to the index page after deleting, but if it is not done, we display an error message instead of raising an error. Remove some redundant imports because the BakcpexWeb html already imports it.
ricmarinovic
changed the title
Flo0807
requested changes
Jun 27, 2025
Comment on lines
+104
to
+156
| defp assign_item_actions(socket) do | ||
| item_actions = Backpex.ItemAction.default_actions() |> socket.assigns.live_resource.item_actions() | ||
| assign(socket, :item_actions, item_actions) | ||
| end | ||
|
|
||
| defp maybe_handle_item_action(socket, key) do | ||
| key = String.to_existing_atom(key) | ||
| action = socket.assigns.item_actions[key] | ||
| item = socket.assigns.item | ||
|
|
||
| if Backpex.ItemAction.has_confirm_modal?(action) do | ||
| open_action_confirm_modal(socket, action, key) | ||
| else | ||
| handle_item_action(socket, action, item) | ||
| end | ||
| end | ||
|
|
||
| defp open_action_confirm_modal(socket, action, key) do | ||
| if Backpex.ItemAction.has_form?(action) do | ||
| changeset_function = &action.module.changeset/3 | ||
| base_schema = action.module.base_schema(socket.assigns) | ||
|
|
||
| metadata = Resource.build_changeset_metadata(socket.assigns) | ||
| changeset = changeset_function.(base_schema, %{}, metadata) | ||
|
|
||
| socket | ||
| |> assign(:form_item, base_schema) | ||
| |> assign(:changeset, changeset) | ||
| else | ||
| assign(socket, :changeset, %{}) | ||
| end | ||
| |> assign(:action_to_confirm, Map.put(action, :key, key)) | ||
| end | ||
|
|
||
| defp handle_item_action(socket, action, item) do | ||
| case action.module.handle(socket, [item], %{}) do | ||
| {:ok, socket} -> | ||
| socket | ||
| |> assign(action_to_confirm: nil) | ||
| |> assign(selected_items: []) | ||
| |> assign(select_all: false) | ||
|
|
||
| unexpected_return -> | ||
| raise ArgumentError, """ | ||
| Invalid return value from #{inspect(action.module)}.handle/3. | ||
| Expected: {:ok, socket} | ||
| Got: #{inspect(unexpected_return)} | ||
| Item Actions with no form fields must return {:ok, socket}. | ||
| """ | ||
| end | ||
| end |
There was a problem hiding this comment.
We already have essentially the same code in index.ex. Could these functions be moved to another module (we might have to make slight adjustments to them to cover both use cases)? What do you think of Backpex.ItemAction?
(Credo also doesn't like the code duplication)
Comment on lines
+49
to
+64
| def handle_event("item-action", %{"action-key" => key}, socket) do | ||
| item = socket.assigns.item | ||
|
|
||
| socket | ||
| |> assign(selected_items: [item]) | ||
| |> maybe_handle_item_action(key) | ||
| |> noreply() | ||
| end | ||
|
|
||
| def handle_event("cancel-action-confirm", _params, socket) do | ||
| socket | ||
| |> assign(:form_item, nil) | ||
| |> assign(:changeset, nil) | ||
| |> assign(:action_to_confirm, nil) | ||
| |> noreply() | ||
| end |
There was a problem hiding this comment.
We might be able to move these functions to Backpex.ItemAction, too (see other comment)
Comment on lines
+410
to
+424
| <div class={["flex items-center justify-end space-x-2"]}> | ||
| <button | ||
| :for={{key, action} <- item_actions_for(:show, @item_actions)} | ||
| :if={@live_resource.can?(assigns, key, @item)} | ||
| id={"item-action-#{key}-#{LiveResource.primary_value(@item, @live_resource)}"} | ||
| type="button" | ||
| phx-click="item-action" | ||
| phx-value-action-key={key} | ||
| aria-label={action.module.label(assigns, @item)} | ||
| phx-hook="BackpexTooltip" | ||
| data-tooltip={action.module.label(assigns, @item)} | ||
| > | ||
| {action.module.icon(assigns, @item)} | ||
| </button> | ||
| </div> |
There was a problem hiding this comment.
I guess, we need another div for the tooltip that has the button inside.
It looks like there is too much space between the text and the tooltip:
| class="h-6 w-6 cursor-pointer transition duration-75 hover:text-primary hover:scale-110" | ||
| /> | ||
| </.link> | ||
| <div class={["flex items-center justify-end space-x-2"]}> |
There was a problem hiding this comment.
Suggested change
| <div class={["flex items-center justify-end space-x-2"]}> | |
| <div class="flex items-center justify-end space-x-2"> |
Comment on lines
+2
to
+22
| <.modal | ||
| :if={@action_to_confirm} | ||
| id="action-confirm-modal" | ||
| title={@action_to_confirm.module.label(assigns, nil)} | ||
| on_cancel={JS.push("cancel-action-confirm")} | ||
| open={true} | ||
| close_label={Backpex.__("Close modal", @live_resource)} | ||
| > | ||
| <div class="px-5 py-3"> | ||
| {@action_to_confirm.module.confirm(assigns)} | ||
| </div> | ||
| <div> | ||
| <.live_component | ||
| module={Backpex.FormComponent} | ||
| id={:item_action_modal} | ||
| live_resource={@live_resource} | ||
| action_type={:item} | ||
| {Map.drop(assigns, [:socket, :flash, :fields])} | ||
| /> | ||
| </div> | ||
| </.modal> |
There was a problem hiding this comment.
We might want to move this to another component as this is duplicated in resource_show.html.heex and resource_index.html.heex
|
Good job and thank you for the contribution! 🙌 I've requested some changes. Also, there are some errors in the CI. @ricmarinovic |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #872.
Changes:
itemtoform_itemin theform_componentto avoid the name clash in thesocket.assigns.item_actionsinresource.exto be more generic and allow it to be used with the newshowparameter.Tested manually, couldn't find a way to easily run the tests for the
demoapplication.