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

data-turbo-permanent doesn't work for frame or stream renders #64

Closed
stgm opened this issue Jan 1, 2021 · 3 comments
Closed

data-turbo-permanent doesn't work for frame or stream renders #64

stgm opened this issue Jan 1, 2021 · 3 comments

Comments

@stgm
Copy link

stgm commented Jan 1, 2021

I'm creating a separate issue for this because @sstephenson mentioned this and it would be great to see this work in the future.

In my case, I have a sidebar where users can switch sidebar content using a turbo frame—but the content is not correctly reinstated when navigating back and forth using turbo drive.

Originally posted by @sstephenson in #21 (comment)

data-turbo-permanent has not been removed, it just doesn't (currently) work for frame or stream renders.

@stgm
Copy link
Author

stgm commented Jan 1, 2021

Here's a simplified version of my code as an example:

<div id="sidebar-content" data-turbo-permanent>
    <turbo-frame id="sidebar-frame" target="_top">
        <%= button_to '<', schedule_prev_path, form: { data: { 'turbo-frame': 'sidebar-frame' } } %>
        <%= button_to '>', schedule_next_path, form: { data: { 'turbo-frame': 'sidebar-frame' } } %>
        ... page navigation links ...
    </turbo-frame>
</div>

seanpdoyle added a commit to seanpdoyle/turbo that referenced this issue Jan 4, 2021
Related to hotwired#64

Introduce the `FrameRenderer` (as a descendant of the `Renderer`
interface, inspired by the `SnapshotRenderer` implementation) to serve
as an abstraction for the behavior the `FrameController` was previously
responsible for.

Extract `turbo-frame` content extraction from the `FrameController`
class and combine it with code copied from `SnapshotRenderer`
responsible for cloning permanent elements into the new HTML.

It's likely that there is a concept awaiting extraction, or a less
duplicative way of re-using the permanent element extraction logic, but
this is a start.

Note, this commit does not resolve the `<turbo-stream>` element
rendering shortcomings, but might blaze a trail for a `StreamRenderer`
implementation.
seanpdoyle added a commit that referenced this issue Jan 4, 2021
Related to #64

Introduce the `FrameRenderer` (as a descendant of the `Renderer`
interface, inspired by the `SnapshotRenderer` implementation) to serve
as an abstraction for the behavior the `FrameController` was previously
responsible for.

Extract `turbo-frame` content extraction from the `FrameController`
class and combine it with code copied from `SnapshotRenderer`
responsible for cloning permanent elements into the new HTML.

It's likely that there is a concept awaiting extraction, or a less
duplicative way of re-using the permanent element extraction logic, but
this is a start.

Note, this commit does not resolve the `<turbo-stream>` element
rendering shortcomings, but might blaze a trail for a `StreamRenderer`
implementation.
@stgm
Copy link
Author

stgm commented Jan 6, 2021

My data-turbo-permanent problem appears to be unrelated to turbo-frame after all, see #80

seanpdoyle added a commit to seanpdoyle/turbo that referenced this issue Jan 12, 2021
Related to hotwired#64

Introduce the `FrameRenderer` (as a descendant of the `Renderer`
interface, inspired by the `SnapshotRenderer` implementation) to serve
as an abstraction for the behavior the `FrameController` was previously
responsible for.

Extract `turbo-frame` content extraction from the `FrameController`
class and combine it with code copied from `SnapshotRenderer`
responsible for cloning permanent elements into the new HTML.

It's likely that there is a concept awaiting extraction, or a less
duplicative way of re-using the permanent element extraction logic, but
this is a start.

Note, this commit does not resolve the `<turbo-stream>` element
rendering shortcomings, but might blaze a trail for a `StreamRenderer`
implementation.
seanpdoyle added a commit that referenced this issue Jan 12, 2021
Related to #64

Introduce the `FrameRenderer` (as a descendant of the `Renderer`
interface, inspired by the `SnapshotRenderer` implementation) to serve
as an abstraction for the behavior the `FrameController` was previously
responsible for.

Extract `turbo-frame` content extraction from the `FrameController`
class and combine it with code copied from `SnapshotRenderer`
responsible for cloning permanent elements into the new HTML.

It's likely that there is a concept awaiting extraction, or a less
duplicative way of re-using the permanent element extraction logic, but
this is a start.

Note, this commit does not resolve the `<turbo-stream>` element
rendering shortcomings, but might blaze a trail for a `StreamRenderer`
implementation.
seanpdoyle added a commit to seanpdoyle/turbo that referenced this issue Jan 12, 2021
Related to hotwired#64

Introduce the `FrameRenderer` (as a descendant of the `Renderer`
interface, inspired by the `SnapshotRenderer` implementation) to serve
as an abstraction for the behavior the `FrameController` was previously
responsible for.

Extract `turbo-frame` content extraction from the `FrameController`
class and combine it with code copied from `SnapshotRenderer`
responsible for cloning permanent elements into the new HTML.

It's likely that there is a concept awaiting extraction, or a less
duplicative way of re-using the permanent element extraction logic, but
this is a start.

Note, this commit does not resolve the `<turbo-stream>` element
rendering shortcomings, but might blaze a trail for a `StreamRenderer`
implementation.
seanpdoyle added a commit to seanpdoyle/turbo that referenced this issue Jan 12, 2021
Related to hotwired#64

Introduce the `FrameRenderer` (as a descendant of the `Renderer`
interface, inspired by the `SnapshotRenderer` implementation) to serve
as an abstraction for the behavior the `FrameController` was previously
responsible for.

Extract `turbo-frame` content extraction from the `FrameController`
class and combine it with code copied from `SnapshotRenderer`
responsible for cloning permanent elements into the new HTML.

It's likely that there is a concept awaiting extraction, or a less
duplicative way of re-using the permanent element extraction logic, but
this is a start.

Note, this commit does not resolve the `<turbo-stream>` element
rendering shortcomings, but might blaze a trail for a `StreamRenderer`
implementation.
seanpdoyle added a commit to seanpdoyle/turbo that referenced this issue Jan 22, 2021
Related to hotwired#64

Introduce the `FrameRenderer` (as a descendant of the `Renderer`
interface, inspired by the `SnapshotRenderer` implementation) to serve
as an abstraction for the behavior the `FrameController` was previously
responsible for.

Extract `turbo-frame` content extraction from the `FrameController`
class and combine it with code copied from `SnapshotRenderer`
responsible for cloning permanent elements into the new HTML.

It's likely that there is a concept awaiting extraction, or a less
duplicative way of re-using the permanent element extraction logic, but
this is a start.

Note, this commit does not resolve the `<turbo-stream>` element
rendering shortcomings, but might blaze a trail for a `StreamRenderer`
implementation.
@stgm
Copy link
Author

stgm commented Jan 30, 2021

@sstephenson I think this one was also resolved? I created the issue based on your comment, but I do not actually have this use case after all.

@stgm stgm closed this as completed Jan 30, 2021
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

1 participant