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

feat(hx-trigger): add support for rootMargin on intersect #2593

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

believer
Copy link

@believer believer commented Jun 5, 2024

Description

Add support for rootMargin in intersect triggers. This allows us to extend the size of the intersect area. I've selected to separate multiple values using underscores, similar to how Tailwind solves it.

Corresponding issue: #1349

Testing

I've tested it locally using one of my own projects. I didn't find any tests for the intersect modifiers, so I couldn't find anything to base new tests on.

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 Jun 6, 2024
@1cg
Copy link
Contributor

1cg commented Jun 27, 2024

i like everything about this except the underscores. What if we consumed until EOF or the next token followed by a colon? (this may be a horrible idea idk)

@believer
Copy link
Author

I wasn't too excited about the underscores either, but I didn't have a better idea at the time. Two other options could be:

  • /,|(root)|(threshold)/ to match comma or one of the other two intersect options
  • /[,rt]/ for comma and the starting character of the options

I think the first option is easier to understand though

@believer
Copy link
Author

believer commented Sep 9, 2024

@1cg any thoughts on the updated parsing proposal above?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants