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.

Sign up for GitHub

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

Jump to bottom

Figure out hot to handle shadow_root and shadow_dom_target in non-svelthree 'container' components #72

Closed
vatro opened this issue Jun 13, 2022 · 4 comments
Closed

Figure out hot to handle shadow_root and shadow_dom_target in non-svelthree 'container' components #72

vatro opened this issue Jun 13, 2022 · 4 comments
Assignees
@vatro
Labels
DX general General issue new feature New feature or request shadow DOM
Milestone
1.0.0-next.1

Comments

@vatro
Copy link
Owner

vatro commented Jun 13, 2022 •
edited
Loading

Given:

<!-- Car.svelte (non-svelthree 'container' component with a svelthree 'wrapper' component child having slots populated by non-svelthree containers) -->

<Empty name="car_wrapper" >
  <Body name="car_body" />
  <Wheels name="car_wheels" />
</Empty>

<!-- Body.svelte (non-svelthree 'container' component with a svelthree child) -->

<Mesh name="car_body" />

<!-- Wheels.svelte (non-svelthree 'container' component with svelthree children) -->

<Mesh name="wheel_1"/>
<Mesh name="wheel_2"/>
<Mesh name="wheel_3"/>
<Mesh name="wheel_4"/>

<!-- App.svelte -->

<Canvas>
  <Scene>
    <Car />
  </Scene>
</Canvas>

Problem:

svelthree/src/components/Mesh.svelte

Lines 342 to 347 in 07120b0

/**
* IMPORTANT TODO TOFIX \
* if we're combining components into a non-svelthree component, like e.g. a `Car`
* component, there will be no `shadow_dom_target` or `shadow_root_el!
*/
export let shadow_dom_target: SvelthreeShadowDOMElement = undefined

@vatro vatro added new feature New feature or request DX general General issue labels Jun 13, 2022
@vatro vatro added this to the 1.0.0-next.1 milestone Jun 13, 2022
@vatro vatro self-assigned this Jun 13, 2022
@vatro vatro mentioned this issue Jun 13, 2022
Open
17 tasks
@vatro vatro modified the milestones: 1.0.0-next.1, 1.0.0-next.2 Jun 13, 2022
@vatro
Copy link
Owner Author

vatro commented Jun 15, 2022 •
edited
Loading

Turns out, the "problem" has something to do with Svelte. If a non-svelthree 'container' component has a child component with slots and bind:this is set on any of it's slots, the child component will receive context before anything else, even before the Scene which is actually it's higher parent -> means: the Scene component will have no shadow_dom_target which get's generated reactively when the Scene receives the shadow_root (store) / $shadow_root.element shared via context by Canvas.

I've filed a Svelte issue concerning this, but I guess I'll have to find some workaround, like... simply avoiding slots / referencing slots inside non-svelthree 'containers' / 'wrappers'? 🤔 ... no.

@vatro
Copy link
Owner Author

vatro commented Jun 15, 2022 •
edited
Loading

ok, this is a good DUCKTAPE-FIX candidate:

  • put shadow dom target creation into an async function
$: if (shadow_root_el && mesh && !shadow_dom_target) create_shadow_dom_target()
  • wait for tick() before appending the created shadow DOM element:
async function create_shadow_dom_target() { 
... 
  // DUCKTAPE  getContext wrong order fix
  await tick()

  const parents_shadow_dom_target = our_parent.userData.svelthreeComponent.shadow_dom_target

  // debug
  if(!parents_shadow_dom_target) {
    console.error("Mesh >  ...after tick: couldn't find 'parents_shadow_dom_target'!", parents_shadow_dom_target)
  } else {
    console.log("Mesh > ...after tick:", {parents_shadow_dom_target})
  }
...
}

has to be applied to all components generating shadow DOM elements.

@vatro vatro closed this as completed in 42c17a8 Jun 16, 2022
@vatro
Copy link
Owner Author

vatro commented Jun 19, 2022 •
edited
Loading

Unfortunately this is still not 100% fixed...

After fiddling a bit, I think the real fix would be to share parents_shadow_dom_target in a store via context and let the children reactively generate their shadow_dom_target when parents_shadow_dom_target is available.

Removing await tick() from Scene component's logic made it look good for the most of the test-scenes, even scene-in-scene, but the shadow DOM structure of scene-in-mesh was wrong, means this is not the fix.

@vatro vatro reopened this Jun 19, 2022
@vatro vatro modified the milestones: 1.0.0-next.2, 1.0.0-next.1 Jun 19, 2022
@vatro
Copy link
Owner Author

vatro commented Jun 20, 2022 •
edited
Loading

Ok, will deploy the new fix in a minute.
I ended up using context only (not stores) close to the logic for getting parent components / scenes.
Checked with test-scenes, looking good.

vatro pushed a commit that referenced this issue Jun 20, 2022
Rename shadow_dom_target to shadow_dom_el
d39342c 
Preparation for fixing #72
@vatro vatro closed this as completed in e56fc3e Jun 20, 2022
vatro pushed a commit that referenced this issue Jun 20, 2022
Fix LoadedGLTF
2475096 
was missing 'a bit' of code 😬 -> #72
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@vatro vatro

Labels
DX general General issue new feature New feature or request shadow DOM
Projects
None yet
Milestone
1.0.0-next.1
Development

No branches or pull requests
1 participant
@vatro