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.

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

[Trestle 0.9.9] Custom post/patch action in a dialog form does not update the form in place #466

Open
GabGab opened this issue Jul 4, 2024 · 7 comments

Comments

@GabGab
Copy link

GabGab commented Jul 4, 2024

Hello everyone,

I'm on a Rails 6 project (Ruby 2.6.5), with Trestle now updated to 0.9.9 (from 0.8.13), Trestle-search 0.4.3 and Trestle-auth 0.4.4
I'm testing with Chrome 126.0.6478.62

I have a User model and am using Trestle to let an admin create/update users.
In the form, which has the dialog: true option, i have a link_to link with remote: true that performs a custom_action defined in UsersAdmin.

When i click on this link :

  • On Trestle 0.8.13, the server is hit correctly, updates the User correctly, responds with a redirection to the instance edit action, and the dialog is updated accordingly (with a flash and the generated QR code in my example), as per those 2 screenshots

Capture d’écran 2024-07-04 à 14 14 20

Capture d’écran 2024-07-04 à 14 14 28

  • On Trestle 0.9.9, the server is hit correctly as well, updates the User correctly, responds with the same redirection, but the dialog is not updated. It feels like nothing happened for the admin who clicked on the button. When reloading the page and the form, the correct updated dialog is displayed.

This only happens for extra links in the form calling custom actions, the basic form submission works perfectly with dialogs. I've taken a look at the Trestle code, and i found the code that replaces the dialog content on form submission (in form.js), but I can't explain why the working behavior of 0.8.13 is not working anymore for me on 0.9.9 .

Without the context of a dialog form, removing the remote: true from the link makes everything work with no dialog. However i wish to use a dialog in this scenario.

To reproduce, simply use a standard Trestle admin, create a record for it, and add these peaces of code :

form dialog: true do |model|
  row do
      col(xs: 12) do
          concat link_to("Custom button", admin.path(:custom_action, id: model), method: :post, class: "btn btn-success", remote: true)
      end
   end
end

controller do
    def custom_action
      model = admin.find_instance(params)

      flash[:message] = "The custom_action was called, here's a flash message !"
      redirect_to admin.instance_path(model, :edit)
    end
end

routes do
    post :custom_action, on: :member
end

Are you able to reproduce this ? If so, do you know how to update the dialog after a remote custom_action ?

Thank you for your help !

@spohlenz
Copy link
Member

spohlenz commented Jul 4, 2024

I'm able to reproduce this behavior (both the old that's working and the new that's not). Thank you for the clear reproduction example.

At this stage, I can't see why 0.8.13 works and 0.9.9 doesn't, as most of the form and dialog code is fairly similar between those versions. I'm more surprised that this does work in 0.8.13 -- you don't have any custom JS handling the ajax:success event that isn't being wired up in 0.9.9 perhaps?

My only other thought is that it's maybe a change in behavior between jquery-ujs (used in 0.8.x) and rails-ujs (used in 0.9.x) but nothing is jumping out at me there either.

@GabGab
Copy link
Author

GabGab commented Jul 4, 2024

Thank you for your quick reply !
I've reached the same surprise as you, the code around the form and dialogs does not seem to have changed a lot over the years ...

As it stands i have no relevant extra custom JS on top of Trestle :/

My only solution here seems to be to remove the dialog behavior altogether from the forms with custom actions
Thank you for taking the time to look into it :)

@GabGab
Copy link
Author

GabGab commented Jul 4, 2024

@spohlenz do you think you'll have some time to investigate further into the regression ? :)

@spohlenz
Copy link
Member

spohlenz commented Jul 5, 2024

I'd definitely like to at least understand why this is happening (and hopefully find a workaround for you). I'll aim to investigate further over the next few days.

Having said that, the majority of my focus is on the new 0.10.0 release which has Turbo & Stimulus built-in, and should hopefully make these types of updates much easier. If you feel inclined, you may wish to experiment with 0.10.0.pre. I'm aiming to put together some examples of exactly these types of non-standard updates using Turbo and the Stimulus controllers built in to Trestle 0.10.0.

@GabGab
Copy link
Author

GabGab commented Jul 5, 2024

@spohlenz thanks for your quick reply, and the investigation :)

If you happen to find a way to work around that problem with 0.9.9, i'd be happy to hear it !

In the meantime i will also try to update to 0.10.0.pre and see if i have a more satisfactory dialog behavior

@spohlenz
Copy link
Member

spohlenz commented Jul 7, 2024

I've been able to identify the line of code causing the difference in behavior:
https://github.com/TrestleAdmin/trestle/blob/v0.9.9/frontend/js/components/form.js#L36-L37

Your data-remote link within the form triggers the ajax:complete events that were previously caught by the Trestle form event handlers. In 0.9.x, I added an explicit check to only catch errors on the form itself, and not those bubbled up from within (as this can cause unexpected behavior in other situations).

If you'd like to do something similar in 0.9.x, you can add the following code to app/assets/javascripts/trestle/custom.js:

Trestle.init(function (root) {
  $(root).find('[data-behavior="update-dialog"]')
    .on('ajax:send', function (e) {
      $(this).addClass('loading');
    })
    .on('ajax:complete', function (e) {
      var xhr = e.detail[0];

      $(this).removeClass('loading');

      var contentType = (xhr.getResponseHeader('Content-Type') || '').split(';')[0];

      if (contentType === 'text/html') {
        if (/<html/i.test(xhr.responseText)) {
          // Response is a full HTML page, likely an error page. Render within an iframe.
          var $context = $(this).closest('[data-context]');
          var iframe = $('<iframe>').addClass('error-iframe').get(0);
          $context.html(iframe);

          iframe.contentWindow.document.documentElement.innerHTML = xhr.responseText;
        } else {
          // Find the parent context and replace content
          var $context = $(this).closest('[data-context]');
          $context.html(xhr.responseText);

          // Initialize replaced elements within the context
          Trestle.triggerInit($context);

          // Focus the correct tab
          Trestle.focusActiveTab();
        }
      } else if (contentType === 'text/plain') {
        // Assume an error response
        const title = xhr.status + '(' + xhr.statusText + ')';
        Trestle.Dialog.showError(title, xhr.responseText);
      }
    });
});

and add data: { behavior: "update-dialog" } to your link/button.

I'll update this thread tomorrow with an example of how to do this in 0.10.0.

@GabGab
Copy link
Author

GabGab commented Jul 8, 2024

Hello again @spohlenz ,
Thank you very much for taking the time to look into it, and well done for finding the culprit line, your custom fix does the job perfectly !

It might even be worth it to implement it as a feature in 0.10.0 ?

Thanks again and have a great day :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants