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

Allow removing extension inheritance with this-only: option #3020

Open
wants to merge 7 commits into
base: dev
Choose a base branch
from

Conversation

MichaelWest22
Copy link
Contributor

@MichaelWest22 MichaelWest22 commented Nov 14, 2024

Description

Allow hx-ext extension inheritance to be disabled on an element with a new this-only: prefix instead of having to use ignore: on all children.

It works by setting isParentScan to track when scanning parent nodes recursively and then ignoring recursive calls that have ignore: prefix. Otherwise it strips the ignore: prefix.

Also improved readability/performance/minification by:
removing un-needed explicit undefined checks
moving replace => trim
moving slice => indexOf

Corresponding issue: #3016

Testing

Added test to confirm extension with this-only: prefix does not get inherited as expected.

Checklist

  • I have read the contribution guidelines
  • I have targeted this PR against the correct branch (master for website changes, dev for
    source changes)
  • This is either a bugfix, a documentation update, or a new feature that has been explicitly
    approved via an issue
  • I ran the test suite locally (npm run test) and verified that it succeeded

@Telroshan Telroshan added the enhancement New feature or request label Nov 14, 2024
@Telroshan Telroshan added the ready for review Issues that are ready to be considered for merging label Nov 14, 2024
@Telroshan
Copy link
Collaborator

Not a change request at all, just throwing out alternative naming ideas for the prefix itself in case you think they sound good ; self:, isolate:, or isolated: came to mind while answering on the issue

@MichaelWest22
Copy link
Contributor Author

MichaelWest22 commented Nov 17, 2024

Yeah local: is not the perfect word for this but I've struggled to find a better word that fits as well. local brings to mind things like local vs global scope which is well understood but in a way the current extension default behavior is already locally scoped to the form element! it just also applies to the form elements children because they are in the same scope normally. But in this use case we want to just break inheritance to the children which makes self: make more sense. isolate: and isolated: have the issue that is not obvious what you are isolating without reading the documentation as i think these are less self explaining than 'local:'.

What i like about local: is that how it reads in your head when looking at hx-ext="local:my-exetnsion" as it makes you think "my-extension is a local extension that only applies locally". But something like self: while possibly more accurate you have to think hx-ext="self:my-extension" means "my-extension is an extension that only applies to itself where itself refers to the local element it is tagged on".

There are other more complex options that are even more accurate and descriptive like disinherit: (mirror hx-disinherit attribute) or noninheritable: or nonheritable: . But the disinherit one feels more like the ignore: case and not for setting the extension up situation. The other two *heritable ones seem to be too long and complicated to use to me.

@MichaelWest22 MichaelWest22 changed the title Allow removing extension inheritance with local: option Allow removing extension inheritance with this-only: option Dec 5, 2024
@MichaelWest22
Copy link
Contributor Author

@Telroshan I just tried updating this change to use this-only: instead of local: to see if this improves the readability and understandability of this feature proposal. asked for some advice on discord and had suggestions for using self-only or this as good options. I like this because it has existing meaning and use in htmx like in hx-target to refer to the element itself. self is also good but its not as obvious if the subject of the self is the element or the extension so I think this is probably more accurate. Adding the -only makes it much more obvious and human readable I think as it now reads like apply this extension to this element only

@Telroshan
Copy link
Collaborator

Sounds like a good name to me!

Copy link
Contributor

@1cg 1cg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one small change thank you!

src/htmx.js Outdated Show resolved Hide resolved
@xhaggi
Copy link
Contributor

xhaggi commented Dec 12, 2024

As I said before, I'm not a big fan of these prefixes. Introducing attributes such as hx-ext-ignore and hx-ext-disinherit are the better choice in my opinion. We still have the choice now, once this change is in, it will be hard to change it.

<form action="/action" hx-ext="ext1,ext2" hx-ext-disinherit="ext1"> <!-- disable inheritance for ext1 -->
  <a href="#" hx-delete="/delete"></a>
</form>
<form action="/action" hx-ext="ext1,ext2">
  <a href="#" hx-get="/get" hx-ext-ignore="ext1,ext2"></a> <!-- ignore ext1,ext2 only on this element -->
  <a href="#" hx-delete="/delete"></a>
</form>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request ready for review Issues that are ready to be considered for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants