bigskysoftware / htmx

</> htmx - high power tools for HTML
https://htmx.org
Other
38.03k stars 1.29k forks source link

Extended CSS selectors find targets relative to the issues, not the attribute #1841

Open tim-timman opened 1 year ago

tim-timman commented 1 year ago

The issue

Using hx-target with extended CSS selectors are not selecting targets relative to the hx-target attribute, but instead relative to the "issuer" element (one with an hx-get for example). I find this being unintuitive and makes the extended selectors less useful.

Example

<div hx-target="next .target">
  <p class="target">Target</p>
  <button hx-get="/request">Make request</button>
</div>

Expectation

Clicking the button would update Target with the result of the request. As it would find the ancestor's hx-target, where the next .target is Target.

Reality

Clicking the button to make the request results in a htmx:targetError, because it uses the ancestor's hx-target selector next .target but does the search from the button—hence doesn't find one.

Fix

Make the search relative to the where the used hx-target element. I can't think of a case where the current implementation is preferable.

Code to easily reproduce

Here's a Python script to spin up a modified SimpleHTTPServer to serve the an example of the expected behavior more visually. Using bulma stylesheet to make it a bit more easier on the eyes–looking like this: image

Save and run with python3.

from http.server import SimpleHTTPRequestHandler, HTTPServer
import os
import secrets
from tempfile import NamedTemporaryFile
from urllib.request import urlopen
import weakref
import webbrowser

virtual_files = {}
def add_response(file_extension, contents, path=None):
    """convenience to fake making a file, served on a path"""
    path = path or f"/{secrets.token_urlsafe(8)}"
    with NamedTemporaryFile("wb", suffix=f".{file_extension}", delete=False) as f:
        f.write(contents if isinstance(contents, bytes) else contents.encode())
        virtual_files[path] = f
        weakref.finalize(f, os.unlink, f.name)
    return path

HTMX_URL = "https://raw.githubusercontent.com/bigskysoftware/htmx/ec126d2e3d973325f53fc69ed591ee908901e839/dist/htmx.js"

add_response("html", path="/", contents=f"""\
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8" />
<title>htmx bug reproduction</title>
<link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/bulma@0.9.4/css/bulma.min.css">
<script src="{add_response("js", urlopen(HTMX_URL).read())}"></script>
</head>
<body class="section">
<div class="container block">

<!-- REPRODUCTION -->

<div class="box content">
    <p>Example from issue; see <code>htmx:targetError</code> in DevTools</p>
    <div hx-target="next .target">
      <p class="target">Target</p>
      <button hx-get="{add_response("html", 'Success')}">Make request</button>
    </div>
</div>

<hr/>

<div class="box content" hx-target="next .message-body" hx-swap="beforeend">
    <p>This box has <code>hx-target="next .message-body"</code></p>
    <hr/>
    <article class="message is-primary is-light">
        <div class="message-header">.message-body #1</div>
        <div class="message-body"></div>
    </article>
    <div class="columns block content">
        <div class="column">
            <p>This button doesn't have <code>hx-target</code>; will use box's <code>hx-target</code></p>
            <button class="button is-primary is-light"
                    hx-get="{add_response("html", '<p class="has-background-primary-light">Without hx-target result</p>')}"
            >Make request</button>
        </div>
        <div class="column">
            <p>This button has: <code>hx-target="next .message-body"</code></p>
            <button class="button is-info is-light"
                    hx-get="{add_response("html", '<p class="has-background-info-light">With hx-target result</p>')}"
                    hx-target="next .message-body"
            >Make request</button>
        </div>
    </div>
    <article class="message is-info is-light">
        <div class="message-header">.message-body #2</div>
        <div class="message-body"></div>
    </article>
</div>

<!-- END REPRODUCTION -->

</div>
</body>
</html>
""")

class RequestHandler(SimpleHTTPRequestHandler):
    def translate_path(self, path):
        file = virtual_files.get(path)
        if file is not None:
            return virtual_files[path].name
        return super().translate_path(path)

if __name__ == "__main__":
    server = HTTPServer(server_address=('localhost', 8000), RequestHandlerClass=RequestHandler)
    host, port = server.socket.getsockname()[:2]
    url = "http://{}:{}/".format(f'[{host}]' if ':' in host else host, port)
    print(f"Serving on: {url}")
    webbrowser.open(url)
    server.serve_forever()
andryyy commented 1 year ago

It’s an inherited attribute and therefore virtually added to the element that triggers the event.

Besides that it feels like a "breaking" change to me.

Wouldn’t a hx-target 'closest:.content .message-body' work? As set on .content?

This wouldn’t even break when behavior was changed according to your PR.

tim-timman commented 1 year ago

Wouldn’t a hx-target 'closest:.content .message-body' work? As set on .content?

This wouldn’t even break when behavior was changed according to your PR.

No, that just results in a htmx:targetError, unless I'm misunderstanding what you mean. But it is unchanged with our without my change.

Edit: Accidentally clicked the wrong thing.

tim-timman commented 1 year ago

closest appears to only be able to target ancestors. So although .content .message-body does match from one of the parent nodes, I guess it doesn't match according to "closest" due to the actual matches not being a ancestors.

alexpetros commented 1 year ago

@andryyy Is correct that this would absolutely be a breaking change, and we cannot merge it for this reason. For the time being, I think the best we can do is a documentation change, but we can also discusses whether the behavior you want can be added somehow.