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

Page Refreshes: allow them to happen on an "advance" action #1085

Open
weaverryan opened this issue Nov 28, 2023 · 2 comments
Open

Page Refreshes: allow them to happen on an "advance" action #1085

weaverryan opened this issue Nov 28, 2023 · 2 comments

Comments

@weaverryan
Copy link

Hi!

I've been testing out Turbo 8 because I'm super excited about the page refreshes. One use-case for "page refreshes" is a "data-tables" like setup, where you can search a table, click column links to sort, paginate, etc. Submitting the search form will work great with page refreshes.

But currently, the "column sorting" links and pagination links can't use page refreshes. That's the "action" must be replace:

return !visit || (this.lastRenderedLocation.href === visit.location.href && visit.action === "replace")

This was done on purpose in #1072 for sensible reasons. And there IS a workaround - <a href="..." data-turbo-action="replace">. However, this is a case where I DO want the history to advance so the user can click back to the previous page.

Would it be reasonable to introduce a new data-turbo-morph attribute for the link to opt into morphing? The URL of the page would still need to match, this would just remove the requirement for the action to be replace.

Cheers!

@jorgemanrubia
Copy link
Member

jorgemanrubia commented Dec 2, 2023

I understand the need but I have some concerns with using morphing for regular "advance" navigations.

There is an internal implementation one: current snapshot caching in Turbo won't work, because the current implementation relies on managing different body elements at rendering time (not the same morphed one). That's something we can solve, but, more conceptually, does browser history make sense with morphing? You are changing regions of the screen. Should Turbo create snapshots before each morph so that it can restore those? You wouldn't do that with stream actions, why would you do for page refreshes with morphing?

Also, this would also be an additional API in the library, more choice for the user, which is something that bothers me.

We'll soon support page refreshes with different params in the URL, would that alleviate your use case here? You still wouldn't have browser history, but at least you could have meaningful URLs representing the state of the filter.

@weaverryan weaverryan changed the title Page Refreshes: allow them to happen on a "replace" action Page Refreshes: allow them to happen on an "advance" action Dec 5, 2023
@weaverryan
Copy link
Author

Thanks for the reply!

We'll soon #1079, would that alleviate your use case here?

I saw that PR! That is great, yes ❤️

You still wouldn't have browser history, but at least you could have meaningful URLs representing the state of the filter.

I think this will mostly work, I could live with it, though in a data tables setup, you really should be able to navigate back.

That's something we can solve, but, more conceptually, does browser history make sense with morphing? You are changing regions of the screen

Fair point! Though "you are changing regions of the screen" is case-by-case. In a data tables setup, it's expected that navigation won't change the screen position.

The "data tables" examples is my motivating use-case. It can currently be handled with a frame, though the <form><input type="search"></form> needs to live outside the frame (so navigation doesn't reset the focus or position on that input). And if you have another element outside the frame, then you need streams. It's a GREAT use-case for morphing... except that, ideally, you want the history to advance.

Cheers!

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

No branches or pull requests

2 participants