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

Provide light dom props children #79

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

paull39
Copy link

@paull39 paull39 commented May 10, 2023

This commit provides a resolution for issue #41, applying a strategy similar to the one used in pull request #56. The commit ensures that the children of a component, such as 'FooComponent', are correctly rendered even without using the Shadow DOM.

An example component which should render now correct in the not shadow-dom:

function FooComponent(props) {
    return (
        <Fragment>
            <h1>My Heading</h1>
            {/* Render the children inside this div */}
            <div>{props.children}</div>
        </Fragment>
    );
}

@Schleuse
Copy link

I've played around with this a bunch and this works great 👍

But I have encountered one issue: when I don't explicitly pass shadow: false as the option the child components are getting rendered double.

Testcase

Components:

const ParentComponent: Preact.FunctionalComponent = (props) => {
  return (
    <div>
      <h1>Headline</h1>
      { props.children }
    </div>
  );
}

const ChildComponent: Preact.FunctionalComponent = (props) => {
  return (
    <div>Child</div>
  );
}

Markup:

<x-parent>
    <x-child></x-child>
    <x-child></x-child>
</x-parent>

✅ With explicit shadow: false - Works Great

register(ParentComponent, 'x-parent', [], { shadow: false });
register(ChildComponent, 'x-child', [], { shadow: false });

Results in:

<x-parent>
  <div>
    <h1>Headline</h1>
    <x-child>
      <div>Child</div>
    </x-child>
    <x-child>
      <div>Child</div>
    </x-child>
  </div>
</x-parent>

🚫 Without passing any options - Renders children both in a <slot> and as a direct child

register(ChildComponent, 'x-child', []);

Results in:

<x-parent>
  <div>
    <h1>Headline</h1>
    <slot>
      <x-child>
        <div>Child</div>
      </x-child>
      <x-child>
        <div>Child</div>
      </x-child>
    </slot>
  </div>
  <x-child>
    <div>Child</div>
  </x-child>
  <x-child>
    <div>Child</div>
  </x-child>
</x-parent>

Reason

I think the reason for this is the check for the shadow-Option in Line 202 & Line 206.

Maybe the determination of shadow mode should be consolidated with Line 13 to a local variable and then used throughout:

const isShadow = options ? options.shadow === true : false;

@blopker
Copy link

blopker commented Sep 26, 2023

Would love to get this merged! Just ran into this issue. For now, I've added a global style: [slot] { display: none !important; }, but have to explicitly state all slots:

<x-button>
<span slot="child">MyButton</span>
</x-button>

Would be great to just have:

<x-button>
MyButton
</x-button>

Edit: I've just vendored this change into my codebase. Works great!

@djalmajr
Copy link

Any news about that?

@blopker
Copy link

blopker commented Nov 29, 2023

It's funny, in practice I've found I never actually use this. I always need at least two slots in my CEs, never just one child. I've gone back to the official build for now.

@djalmajr
Copy link

I was wondering if porting such a popular library like Ant Design would be possible using web-components... Here's an example usage: https://codepen.io/djalmajr/pen/OJdoxqQ

But this "little problem" makes it unfeasible.

@roma-glushko
Copy link

Bumped into the same problem with duplicated children components. Any intention to finalize and merge this one?
cc @paull39 @bspaulding

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

Successfully merging this pull request may close these issues.

5 participants