wip: adjustment reason edit with turbo-frame link and turbo template with no index

[chaimann]

Summary

This is a draft PR to collect feedback on the proposed solution. If it looks good, I'll make changes to other routes that open modals in admin.

This addresses #6014 (comment) as a potential solution to the index table being loaded on visiting a route rendered within a modal.

• removes the parts that load data in controller and render a table inside edit component
• adds links to table cells with data-turbo-frame ids
• on link click navigates turbo-frame to provided route, rendering only a turbo-frame tag (with layout: false)
• adds an easy way to set "turbo" actions within a controller, which upon registering will make sure that given actions cannot be performed in a non-turbo-frame request (where Turbo-Frame header is not set), and instead be redirected to index

Checklist

Check out our PR guidelines for more details.

The following are mandatory for all PRs:

• I have written a thorough PR description.
• I have kept my commits small and atomic.
• I have used clear, explanatory commit messages.

The following are not always needed:

• 📖 I have updated the README to account for my changes.
• 📑 I have documented new code with YARD.
• 🛣️ I have opened a PR to update the guides.
• ✅ I have added automated tests to cover my changes.
• 📸 I have attached screenshots to demo visual changes.

[tvdeyen]

I like this very much! I only would like us to still be able to render the new/edit routes.

There seems to be a syntax error, that's why the specs are still failing.

[tvdeyen]

No, we want that these routes still accessible in ie. new tabs or tests w/o javascript.

[tvdeyen]

Test failures fixed in #6048

[tvdeyen]

@chaimann I addressed my feedback in #6046 and cherry picked your commits. If you don't mind I would like to continue on that PR. Thanks again for building the foundation of it.

[chaimann]

@tvdeyen no worries, glad I could help!
I've also been thinking about the ways how to correctly render the edit modal on edit_path visit, and came up with an idea of eagerly loading specific frame, which allows us to render only "index" component to "activate" the turbo-frame that will load a modal for our record (since we already have the empty turbo-frames rendered on index pages). With this approach there is a TurboFrame component that allows modifying its src before the frame is rendered. Maybe it could be helpful for you in your PR as well

The only downside with it is that browser will make two GET requests to /:id/edit route - one fires when we visit /:id/edit (e.g. when opening the link in a new tab), and the second one fires as soon as the page is loaded but this time it's a turbo-frame request that eager loads our modal.

Also, with dedicated edit route there are few issues that I found:

• clicking outside modal won't close the modal
• X does not close the modal as well
• upon update the modal is not closed either

I'm sure, though, that these could be fixed easily

[tvdeyen]

Also, with dedicated edit route there are few issues that I found:

• clicking outside modal won't close the modal
• X does not close the modal as well
• upon update the modal is not closed either

I'm sure, though, that these could be fixed easily

I cannot reproduce those in #6046
Have you checked that one out yet?

[chaimann]

@tvdeyen nope, haven't checked it yet. Just double checked in my branch, the X does not close the modal as well is not reproducible now but the other two are still there. Not sure yet what causes them though

[tvdeyen]

@tvdeyen nope, haven't checked it yet. Just double checked in my branch, the X does not close the modal as well is not reproducible now but the other two are still there. Not sure yet what causes them though

Can you check out my branch and verify if this is still the case for you? It is not for me locally and not in tests (they are all passing). I would prefer to continue the conversation as a review of #6046 as I updated a lot more components already.

Thanks for all the great feedback. This really helps to fix the issues with the new admin

[chaimann]

perfect, let's proceed in #6046

[chaimann]

closing in favour of #6046