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

Add shadowRoot host selector #2866

Merged
merged 1 commit into from
Oct 3, 2024

Conversation

MichaelWest22
Copy link
Contributor

@MichaelWest22 MichaelWest22 commented Sep 4, 2024

Description

In the web component documentation there is a reference to using 'host' to target the web components shadow DOM host element but this was never implemented as a valid target in htmx. So I've implemented this as a valid selector and added tests to confirm swapping it and elements outside the shadow DOM can be targeted for replacement.

There was a similar 'root' one implemented however from my testing this seems to not be usable for two reasons. First is that because the shadow root has a very different type called ShadowRoot which does not support most of the options a Node or Element would support which means swaps to this target fail when trying to add classes to the root. Also target has now been wrapped in asElement() which strips root target. So we should probably remove this non functional root selector as it is not documented anywhere either. The 'root' target could have been used by non web component use but this would make it return Document which is also not a Element so it gets stripped as well making it unusable here as well. So there should be no downside I can see if we remove 'root'.

Corresponding issue:
#2846

Testing

Wrote new tests to confirm it is working as expected plus did some manual testing in a test app to confirm Host replacment was working 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 bug Something isn't working ready for review Issues that are ready to be considered for merging labels Sep 5, 2024
@Telroshan
Copy link
Collaborator

Note: I'm labelling this as a bug, as even though this is technically a new feature/enhancement, it was (and still is) documented as working in our docs

@1cg 1cg merged commit 958fef2 into bigskysoftware:dev Oct 3, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working 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.

3 participants