Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

In layout, multiple slots with name attributes cannot be supported[✨] #103

Closed
miguba opened this issue Dec 15, 2022 · 25 comments
Closed
Assignees
Labels
[STAGE-2] incomplete implementation Remove this label when implementation is complete [STAGE-2] not fully covered by tests yet Remove this label when tests are verified to cover the implementation [STAGE-2] unresolved discussions left Remove this label when all critical discussions are resolved on the issue [STAGE-3] docs changes not added yet Remove this label when the necessary documentation for the feature / change is added [STAGE-3] missing 2 reviews for RFC PRs Remove this label when at least 2 core team members reviewed and approved the RFC implementation

Comments

@miguba
Copy link

miguba commented Dec 15, 2022

Is your feature request related to a problem?

In layout, multiple slots with name attributes cannot be supported.

Describe the solution you'd like

Can multiple slots be supported in the layout?

Describe alternatives you've considered

no idear

Additional context

No response

@manucorporat
Copy link

How would this feature look like? i cant see how named slots are useful in layouts? how pages would specify the named slot?

@dhdemerson
Copy link

dhdemerson commented Jan 22, 2023

I found myself wanting this for a layout that consisted of a sidebar and content area. On child routes I wanted to add sub-navigation specific to the child route to the sidebar while rendering the main child content into the content area.

Without something like named slots in layout, would the recommended pattern be to keep the logic in the parent layout to conditionally render the appropriate sub-navigation based on the active child route?

I'm just learning Qwik so I won't speculate on a possible API (or if it's even a good idea to introduce named layout slots).

Possibly related: QwikDev/qwik#2429

@genki
Copy link

genki commented Feb 15, 2023

I think useContent() can be used for that purpose, but it would be nice if it's possible to use a component instead of the menu.md.
For ex, expected the <Slot name={what} /> in the layout is projected from ./<what>.tsx (if it exists) as like as the <Slot /> is projected from ./index.tsx

@julianobrasil
Copy link

julianobrasil commented Jun 8, 2023

@adamdbradley, @manucorporat I like @genki's suggestion. If we want to make it clear that the content is to be projected to a named slot, some token could be introduced in the name, like we do when we want to use a specific layout (by following the [email protected] pattern).

Maybe for the slots we could use something in that direction, for example using squared brackets:

  • index@layout-name[slot-name].tsx
    • index@default[left-side].tsx
    • index@[footer].tsx
    • index@checkout[header].tsx

@Pterygoidien
Copy link

Hi, same thing : it would be great ! I tried to make a side navbar for custom routes on the left for an educational platform, where the menu would vary depending on the part of the tree of the course (section/chapters/pages), and thought named Slots would be ideal in layout, where we could populate the link in the layout through the Slot identifier. The useContent() needs a lot of workarounds to come to that result (didn't succeed yet).

@dgknca
Copy link

dgknca commented Aug 20, 2023

this is essential. still waiting.

@mhevery
Copy link

mhevery commented Aug 21, 2023

How do other meta-fws solve this problem? Can you point to examples?

@dgknca
Copy link

dgknca commented Aug 21, 2023

How do other meta-fws solve this problem? Can you point to examples?

I wanted to have something like this:

// layout.tsx
export default component$(() => {
  return (
    <>
      <div class='py-5'>
        <Slot name='header' />
        <section>
          <Slot />
        </section>
      </div>
    </>
  );
});
// index.tsx
export default component$(() => {
  return (
    <>
      <div q:slot='header'>header items</div>
      <div>
         article content
      </div>
    </>
  );
});

named slots are not working in layout.

@mhevery
Copy link

mhevery commented Aug 22, 2023

So the issue is show here: https://stackblitz.com/edit/qwik-starter-1pnxsm?file=src%2Froutes%2Findex.tsx

import { Slot, component$ } from '@builder.io/qwik';

export default component$(() => {
  return (
    <Layout>
      <Index />
    </Layout>
  );
});

export const Layout = component$(() => {
  return (
    <>
      <div class="py-5">
        <Slot name="header" />
        <section style={{ border: '1px solid red' }}>
          <Slot />
        </section>
      </div>
    </>
  );
});
// index.tsx
export const Index = component$(() => {
  return (
    <>
      <div q:slot="header">header items</div>
      <div>article content</div>
    </>
  );
});

The basic problem is that the <Index/> gets plated into <Layout/> like this:

<Layout>
  <Index/>
</Layout>

So the q:slot is to low in the hierarchy. If you want them projected it would have to be something like this:

<Layout>
   <div q:slot="header">header items</div>
  <Index/>
</Layout>

And that is not how routes are composed.

I think this would be an issue for other meta-frameworks as well. How do they solve this problem? (I can't think of a way they would solve this)

@genki
Copy link

genki commented Aug 22, 2023

@mhevery
I came up with a workaround that should work, but it doesn't.
https://stackblitz.com/edit/qwik-starter-6udjqi?file=src%2Froutes%2Findex.tsx

If we use the useVisibleTask$ instead of the useTask$ then it works.
https://stackblitz.com/edit/qwik-starter-nxhuhk?file=src%2Froutes%2Findex.tsx

Might found a bug.

@mhevery
Copy link

mhevery commented Aug 22, 2023

@genki

I came up with a workaround that should work, but it doesn't.
https://stackblitz.com/edit/qwik-starter-6udjqi?file=src%2Froutes%2Findex.tsx

Right, this is because the <Header> gets rendered first and is streamed into the HTTP response, and then you change the state of the system after the <Header> has already been sent to the browser. So this is working as intended, but it would be nice to get a warning or something to let you know that you have changed the signal after it was used to render output.

The useVisibleTask$ works because you do the rendering on the client, but I would like to discourage you from using that as you are eagerly executing code on the client.

PS: Sorry, I don't have a good answer for you.

@genki
Copy link

genki commented Aug 23, 2023

@mhevery
Oh, I see. And I fixed the workaround to work fine this time.
https://stackblitz.com/edit/qwik-starter-6udjqi?file=src%2Froutes%2Findex.tsx
As you explained, this trick can be used only for footers and so on :)

@genki
Copy link

genki commented Aug 23, 2023

@mhevery
Finally, I solved this issue. How about do this?
https://stackblitz.com/edit/qwik-starter-l8gnoe?file=src%2Froutes%2Findex.tsx

@mhevery
Copy link

mhevery commented Aug 23, 2023

Great! Can this issue be closed?

@remypar5
Copy link

@mhevery I don't believe it can be closed. What is described here is a workaround. Although it does work, it doesn't look very good and requires some scaffolding per layout and page.

Also a first-time user of Qwik. Loving it so far. I would suggest something that is similar that works out of the box:

// layout.tsx
export default component$(() => {
  return (
    <div class="wrapper">
      <Header />
      <Menu />
      <Slot name="submenu" />
      <Slot />
      <Slot name="sidebar" />
      <Footer />
    </div>
  )
})
// index.tsx
// Applied to the default Slot
export default component$(() => (<p>page content</p>))

// Applied to the named slots, if available
export const layoutSlots = {
  sidebar: () => (<p>sidebar content</p>)),
  submenu: () => (<p>submenu content</p>)),
}

Should a layout's named slot not be defined in the page, it'll just remain empty. If a page defines a named Slot which is not available in the layout, it'll be ignored. In short: "If both are available, assign it to the slot. If either is unavailable, ignore it"

This way we can keep it clean and consistent.

@mhevery mhevery self-assigned this Sep 26, 2023
@mhevery
Copy link

mhevery commented Sep 26, 2023

OK, I like your proposal and agree that this would be a good feature. (Sorry it took me so long to understand what you are asking.)

Any chance you would be up for creating a PR for this? I would be happy to guide you through it.

@remypar5
Copy link

@mhevery Yes, I would love to do that. And get some pointers on where to go. After reading CONTRIBUTING.MD, of course.

@mhevery
Copy link

mhevery commented Oct 2, 2023

@Okm165
Copy link

Okm165 commented Oct 22, 2023

How is this going forward?
Kind of stopping my development for it, no use in finding dirty workarounds

@oqx
Copy link

oqx commented Nov 10, 2023

Just chiming in to +1. The benefit of layout is limited if I'm having to create a supplemental layout component to place inside of each route anyway for multi-slot support. Spreading layout across multiple files is kind of a PITA.

@remypar5
Copy link

Hi, I started to work on this, but I don't seem to have the time anymore. I got to the point where caching the layoutSlots worked fine. That was the easy part. The hard part was to go further up the tree to find a Slot q:name="..." component to assign any cached elements to. Last week I started a new job which consumes my full bandwidth.

@wmertens
Copy link
Member

@mhevery wouldn't it be possible to support this by skipping fragments when resolving slots? Then the <Index /> situation you explain in https://github.com/BuilderIO/qwik/issues/2442#issuecomment-1687240269 would just work?

Maybe you can address this in the v2 branch?

@pustovitDmytro
Copy link

pustovitDmytro commented Jan 21, 2024

Would be great to have the ability of dynamic layouts, like in DocumentHead:

export const layoutSlots = ({ resolveValue }) => {
    const receipt = resolveValue(useRecipesDetails);

    return {
        headerContent : receipt
            ? <HeaderContent receipt={receipt}></HeaderContent>
            : null
    };
};

export const head: DocumentHead = ({ resolveValue }) => {
    const receipt = resolveValue(useRecipesDetails);

    return {
        title : receipt ? receipt.title : 'Tastoria Receipt'
    };
};

@wmertens
Copy link
Member

Actually just looking up the tree might be problematic. How about q:slot="../title" so you specify exactly which parent?
That way it's probably also easier to implement.

@gioboa
Copy link
Member

gioboa commented Oct 14, 2024

We moved this issue to qwik-evolution repo to create a RFC discussion for this.
Here is our Qwik RFC process thanks.

@github-actions github-actions bot added [STAGE-2] incomplete implementation Remove this label when implementation is complete [STAGE-2] not fully covered by tests yet Remove this label when tests are verified to cover the implementation [STAGE-2] unresolved discussions left Remove this label when all critical discussions are resolved on the issue [STAGE-3] docs changes not added yet Remove this label when the necessary documentation for the feature / change is added [STAGE-3] missing 2 reviews for RFC PRs Remove this label when at least 2 core team members reviewed and approved the RFC implementation labels Oct 14, 2024
@QwikDev QwikDev locked and limited conversation to collaborators Oct 14, 2024
@gioboa gioboa converted this issue into discussion #179 Oct 14, 2024
@github-project-automation github-project-automation bot moved this from In Progress (STAGE 2) to Released as Stable (STAGE 5) in Qwik Evolution Oct 14, 2024
@shairez shairez removed this from Qwik Evolution Oct 15, 2024

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
[STAGE-2] incomplete implementation Remove this label when implementation is complete [STAGE-2] not fully covered by tests yet Remove this label when tests are verified to cover the implementation [STAGE-2] unresolved discussions left Remove this label when all critical discussions are resolved on the issue [STAGE-3] docs changes not added yet Remove this label when the necessary documentation for the feature / change is added [STAGE-3] missing 2 reviews for RFC PRs Remove this label when at least 2 core team members reviewed and approved the RFC implementation
Projects
None yet
Development

No branches or pull requests