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 #1019

Merged
merged 17 commits into from
Nov 14, 2023
Merged

Page refreshes #1019

merged 17 commits into from
Nov 14, 2023

Commits on Nov 6, 2023

  1. Page refreshes

    This commit introduces the concept of page refresh. A page refresh
    happens when Turbo renders the current page again. We will offer two new
    options to control behavior when a page refresh happens:
    
    - The method used to update the page: with a new option to use morphing
      (Turbo currently replaces the body).
    
    - The scroll strategy: with a new option to keep it (Turbo currently
      resets scroll to the top-left).
    
    The combination of morphing and scroll-keeping results in smoother
    updates that keep the screen state. For example, this will keep both
    horizontal and vertical scroll, the focus, the text selection, CSS
    transition states, etc.
    
    We will also introduce a new turbo stream action that, when broadcasted,
    will request a page refresh. This will offer a simplified alternative
    to fine-grained broadcasted turbo-stream actions.
    
    Co-Authored-By: Jorge Manrubia <jorge@hey.com>
    afcapel and jorgemanrubia committed Nov 6, 2023
    Configuration menu
    Copy the full SHA
    75519fb View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    305612d View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    0911cf9 View commit details
    Browse the repository at this point in the history
  4. Don't morph frames flagged with "refresh=morph" as part of the full p…

    …age refresh
    
    We will refresh those frames with morphing as part of the page refresh, so it does
    not make sense to morph them also as part of the full page refresh. If we do, we'll
    trigger a manual reload because the complete attribute will get removed. This aligns
    the upstreamed version with the private gem we've been using internally.
    
    This also fixes a couple of issues:
    
    - We don't want to manually reload all the remote turbo-frames, only those that are
    flagged with "refresh=morph". Regular remote frames will get reloaded automatically
    when removing their complete attribute during regular page refreshes.
    
    - Using idiomorph's "innerHTML" was resulting in a turbo-frame nested inside the
    target turbo-frame. I think its semantics is not morphing inner contents from both
    currentElement and newElement, but morphing newElement as the inner contents of currentElement.
    
    See #1019 (comment)
    jorgemanrubia committed Nov 6, 2023
    Configuration menu
    Copy the full SHA
    6ee3d0b View commit details
    Browse the repository at this point in the history
  5. Delegate StreamActions.refresh to Session (#1026)

    The bulk of the `StreamActions.refresh` implementation was reaching
    through the global `window.Turbo` property, which itself was reaching
    through either global variables or the `Session`.
    
    This commit moves the implementation out of the `StreamActions` and into
    a new `Session.refresh(url, requestId)` method. With that change, all
    property access is encapsulated within the `Session`.
    
    To support that change, this commit also introduces the
    `StreamElement.requestId` property to read the `[request-id]` attribute.
    seanpdoyle authored and jorgemanrubia committed Nov 6, 2023
    Configuration menu
    Copy the full SHA
    a155f65 View commit details
    Browse the repository at this point in the history
  6. Only morph the turbo-frame contents, not the frame itself

    I had removed this in #d1935bd15c85e0a8776afccb90393fd378aea2d2 but we do
    need innerHTML so that the outer frame don't get touched. The problem we had
    is that we were nesting turbo-frames, so using .children to only address the
    contents instead.
    jorgemanrubia committed Nov 6, 2023
    Configuration menu
    Copy the full SHA
    541679a View commit details
    Browse the repository at this point in the history
  7. Don't patch fetch globally, import patched function where needed inst…

    …ead.
    
    We'll export a `fetch` function apps can import too. This can be necessary if an app,
    for example, is doing a manual `fetch` and it wants to prevent a reflected broadcast
    triggered by that request.
    
    We'll also make rails/request use the Turbo fetch version when available. That will be
    the preferred form. In general, we don't want apps to care about having to import or
    use Turbo's `fetch`, but it will be available for the cases an app needs it.
    
    We'll also expose the patched version via `Turbo.fetch`.
    
    See discussion #1019 (review)
    jorgemanrubia committed Nov 6, 2023
    Configuration menu
    Copy the full SHA
    a36ee14 View commit details
    Browse the repository at this point in the history
  8. Always morph remote turbo-frames when a page refresh happens

    Instead of flagging the frames you want to morph with an special attribute,
    we are always going to reload and refresh with morphing all the turbo-frames
    in the page.
    
    This simplifies the API as it removes the concern of categorizing remote
    turbo-frames on the programmer side when using page-refreshes.
    
    This is an idea by @afcapel, who raised the concern of the confusion the attribute
    replace=morph caused, and questioned its necessity.
    
    We can bring the old approach back if we find real cases that justify it.
    jorgemanrubia committed Nov 6, 2023
    Configuration menu
    Copy the full SHA
    0c6a95d View commit details
    Browse the repository at this point in the history
  9. Configuration menu
    Copy the full SHA
    ccacd51 View commit details
    Browse the repository at this point in the history
  10. Don't refresh automatically turbo-frames that descend from a [data-tu…

    …rbo-permanent] element
    
    Since we are now reloading all the remote frames in the page, we need to make sure we ignore
    those that are contained in elements to be preserved.
    jorgemanrubia committed Nov 6, 2023
    Configuration menu
    Copy the full SHA
    d7c62bb View commit details
    Browse the repository at this point in the history
  11. Don't add new [data-turbo-permanent] elements when they already exist…

    … in the page.
    
    It can happen that a same element exists but that idiomorph won't match it
    because some JS moved the element to another position. This will handle such
    scenarios automatically.
    jorgemanrubia committed Nov 6, 2023
    Configuration menu
    Copy the full SHA
    0e10077 View commit details
    Browse the repository at this point in the history
  12. Don't reload stimulus controllers after morphing

    There was a flaw in the implementation: we wanted to reload the stimulus controllers
    when their element was effectively morphed because some attribute had changed. Our
    implementation was essentially reloading all the stimulus controllers instead.
    
    But, even if we implemented our original idea, we have changed our mind about it being
    the right call. The heuristic of "reload controllers when some attribute changed" came
    from some tests with legacy controllers that used dom attributes to track certain
    conditions. That doesn't seem like enough justification for the original idea.
    
    In general, you don't want to reload controllers unless their elements get disconnected
    or connected as part of the morphing operation. If it's important for a given controller
    to track changes to the dom, than it should do that (e.g: listening to connection of targets or
    outlets, or just with the mutation observer API), but we can't determine that from the outside.
    
    If we introduce some API here, it will be the opposite: an API to force a "reconnect" during
    morphing, but we need to see a real justification in practice.
    jorgemanrubia committed Nov 6, 2023
    Configuration menu
    Copy the full SHA
    9944490 View commit details
    Browse the repository at this point in the history

Commits on Nov 9, 2023

  1. Respect morphing and scroll preservation settings when handling form …

    …errors
    
    Turbo will render 422 responses to allow handling form errors. A common scenario
    in Rails is to render those setting the satus like:
    
    ```
    render "edit", status: :unprocessable_entity
    ```
    
    This change will consider such operations a "page refresh" and will also consider
    the scroll directive.
    jorgemanrubia committed Nov 9, 2023
    Configuration menu
    Copy the full SHA
    b77d283 View commit details
    Browse the repository at this point in the history
  2. Merge pull request #1061 from hotwired/morph-refreshes-form-errors

    Respect morphing and scroll preservation settings when handling form errors
    jorgemanrubia committed Nov 9, 2023
    Configuration menu
    Copy the full SHA
    ddfe38b View commit details
    Browse the repository at this point in the history

Commits on Nov 13, 2023

  1. Morph remote turbo frames where the src attribute has changed

    There are some cases when we don't want to reload a remote turbo frame on
    a page refresh.
    
    This may be because Turbo has added a src attribute to the turbo frame
    element, but we don't want to reload the frame from that URL.
    
    Form example, when a form inside a turbo frame is submitted, Turbo adds
    a src attribute to the form element. In those cases we don't want to
    reload the Turbo frame from the src URL. The src attribute points to the
    form submission URL, which may not be loadable with a GET request.
    
    Same thing can happen when a link inside a turbo frame is clicked. Turbo
    adds a src attribute to the frame element, but we don't want to reload the
    frame from that URL.
    
    If the src attribute of a turbo frame changes, this signals that the server
    wants to render something different that what's currently on the DOM, and
    Turbo should respect that.
    
    This also matches the progressive enhancement behavior philosophy of Turbo.
    The behaviour results in the Turbo frame that the server sends, which
    is what would happen anyway if there was no morphing involved, or on a first
    page load.
    afcapel committed Nov 13, 2023
    Configuration menu
    Copy the full SHA
    cf56cfb View commit details
    Browse the repository at this point in the history
  2. Merge remote-tracking branch 'origin/main' into morph-refreshes

    * origin/main:
      Revert disk cache store (#1062)
    afcapel committed Nov 13, 2023
    Configuration menu
    Copy the full SHA
    022c1df View commit details
    Browse the repository at this point in the history

Commits on Nov 14, 2023

  1. Merge remote-tracking branch 'origin/main' into morph-refreshes

    * origin/main:
      Pass session instance to cache constructor (#1063)
    afcapel committed Nov 14, 2023
    Configuration menu
    Copy the full SHA
    bdee6f4 View commit details
    Browse the repository at this point in the history