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

support in-element & wormhole #2402

Merged
merged 1 commit into from
Aug 22, 2023

Conversation

patricklx
Copy link
Collaborator

@patricklx patricklx commented Jun 2, 2023

this injects a InElement component into the component tree, which has its bounds set to the target. it also adds it as a root tree element, so the click & hover work correctly.
This is also done for EmberWormhole, the code checks for the EmberWormhole instance and changes the bounds to its target element

resolves #1255
closes #1375

@patricklx
Copy link
Collaborator Author

@RobbieTheWagner this is ready for review

Copy link
Member

@RobbieTheWagner RobbieTheWagner left a comment

Choose a reason for hiding this comment

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

This seems okay to me, I just wonder if there is anything built in to the internals of glimmer or modifiers that could make this easier? @wycats @chancancode @MelSumner @kategengler who is working on things related to this? Would love to get a thumbs up from them that this is the best way to do things.

@patricklx patricklx force-pushed the support-in-element branch 2 times, most recently from 57daa37 to b69aa68 Compare June 7, 2023 07:22
Copy link
Contributor

@NullVoxPopuli NullVoxPopuli left a comment

Choose a reason for hiding this comment

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

lgtm, though, I'd like to see it tested out first -- got a demo / gif or something of before/after?

@patricklx
Copy link
Collaborator Author

lgtm, though, I'd like to see it tested out first -- got a demo / gif or something of before/after?

Not at the moment, in holidays...
There are tests for it. Or you can download the artifact and test it :)

@patricklx patricklx force-pushed the support-in-element branch 4 times, most recently from 6588f94 to d6faf27 Compare June 20, 2023 08:41
@RobbieTheWagner
Copy link
Member

@NullVoxPopuli I am sure this is functionally fine, I just wanted to make sure there wasn't an easier glimmer API we could hook into or something because this all seems very manual.

@RobbieTheWagner
Copy link
Member

Since there has been no response from stakeholders, I am going to merge this. If there is a better way, we can always update things later. Thanks for the PR @patricklx!

@RobbieTheWagner RobbieTheWagner merged commit e6b2973 into emberjs:main Aug 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot select any element with an in-element block.
4 participants