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

Add override and default options for cumulative configs #1692

Open
brillout opened this issue Jun 12, 2024 · 18 comments
Open

Add override and default options for cumulative configs #1692

brillout opened this issue Jun 12, 2024 · 18 comments
Labels

Comments

@brillout
Copy link
Member

brillout commented Jun 12, 2024

Basics

Add two new settings default: boolean | string and override: boolean | string for controlling cumulative values:

// pages/+config.js

import { Layout } from './Layout'

export default {
  Layout: {
    value: Layout,
    // Defining another layout will override this layout
    default: true
  }
}
// pages/admin-panel/+config.js

import { Layout } from './Layout'

export default {
  Layout: {
    value: Layout,
    // Override all layouts (only ancestors in the config inheritance tree)
    override: true,
    // Override all layouts (even descendants in the config inheritance tree)
    override: 'ALL'
  }
}
// pages/some-page/+config.js

export default {
  // Remove inherited layouts
  Layout: null
  // Remove all layouts
  Layout: {
    value: null,
    override: 'ALL'
  }
}
// pages/+config.js

import HeadDefaultOpenGraph from './HeadDefaultOpenGraph'
import HeadGlobal from './HeadGlobal'

export default {
  Head: [
    {
      //
      value: HeadGlobal
    },
    {
      value: HeadDefaultOpenGraph,
      default: 'open-graph'
    }
  ]
}

Groups

// pages/+config.js

import HeadDefaultOpenGraph from './HeadDefaultOpenGraph'
import HeadGlobal from './HeadGlobal'

export default {
  Head: [
    {
      // Applies to all pages
      value: HeadGlobal
    },
    {
      value: HeadDefaultOpenGraph,
      // Enable pages to override this value by using the 'open-graph' key
      default: 'open-graph' // can be any string (it's used as a unique identifying key)
    }
  ]
}
// pages/some-page/+config.js

import HeadOpenGraph from './HeadOpenGraph'

export default {
  Head: [
    {
      value: HeadOpenGraph,
      // Only override 'open-graph' group
      override: 'open-graph'
    }
  ]
}

Feedback

If you want this, add a comment below elaborating on why you need this.

@brillout brillout added the enhancement ✨ New feature or request label Jun 12, 2024
@bighorik
Copy link

I would like to be able to flexibly configure this inheritance.
In my practice, a situation often happens when, for example, there are three pages:
applications/listing
applications/favorite
applications/create

For the first two, you need to use a common complex Layout, for the latter, a simpler layout. Accordingly, I would like to be able to set my own Layout for /create, overwriting the original one.

However, there are also reverse situations when you need to specify the Layout for 1 of the many pages, for example:
personalAccount/profile
personalAccount/settings
personalAccount/companies/portfolio
personalAccount/companies/stats
personalAccount/companies/about

In this case, I have added additional navigation for pages /companies - the ability to inherit previous Layouts is great

@brillout
Copy link
Member Author

@bighorik Yeah, currently the only way is to do this:

applications/(complex-layout)/listing
applications/(complex-layout)/favorite
applications/(simple-layout)/create

Let me know whether it's a showstopper for you and I'll bump the priority of this ticket.

@AgentEnder
Copy link

AgentEnder commented Jul 17, 2024

I'd love a way to "escape" from a parent layout defined in the renderer. In my app, I have

pages/{a bunch of pages}
pages/presentations/view
renderer/+Layout.jsx

pages/presentations/view needs to not have a surrounding layout, and currently I don't see a way to get it to not be present.

I have a workaround in renderer's layout, but I don't like it:

export function Layout({ children }: PropsWithChildren) {
  const pageContext = usePageContext();

  return pageContext.urlPathname.includes('presentations/view') ? (
    <MinimumPageShell pageContext={pageContext as any}>
      {children}
    </MinimumPageShell>
  ) : (
    <PageShell pageContext={pageContext as any}>{children}</PageShell>
  );
}

Moving the presentations/view page away from presentations/index and similar would feel weird to workaround the layout inheritance

@brillout
Copy link
Member Author

@AgentEnder Makes sense. I think this would be nice:

// pages/some-page/+config.js

export default {
  // Remove all inherited layouts
  Layout: null
}

@AgentEnder
Copy link

That could work, currently I divide my PageShell component into MinimumPageShell and the base PageShell as saw above. Ideally I'd be able to set a Layout that is just the MinimumPageShell

@brillout brillout changed the title [Cumulative configs] New settings override and default Add override and default options for cumulative configs Jul 25, 2024
@bighorik
Copy link

bighorik commented Oct 3, 2024

@bighorik Да, в настоящее время единственный способ - сделать это:

applications/(complex-layout)/listing
applications/(complex-layout)/favorite
applications/(simple-layout)/create

Дайте мне знать, подходит ли вам это для показа, и я увеличу приоритет этого запроса.

Unfortunately, this is not suitable. I have a layout common to all pages, which at some levels of nesting of the file system must be completely overwritten. I'm using vike version 0.4.171 in production, and the lack of backward compatibility makes me cry :(

@brillout
Copy link
Member Author

brillout commented Oct 4, 2024

Unfortunately, this is not suitable. I have a layout common to all pages, which at some levels of nesting of the file system must be completely overwritten.

Can you elaborate?

I'm using vike version 0.4.171 in production, and the lack of backward compatibility makes me cry :(

The 0.4.x release branch is backwards compatible. If it isn't then I consider it a bug; elaborate if that's the case.

@alexmnv
Copy link

alexmnv commented Oct 14, 2024

@bighorik Yeah, currently the only way is to do this:

applications/(complex-layout)/listing
applications/(complex-layout)/favorite
applications/(simple-layout)/create

That seems a bit excessive if there's only one page that requires a specific layout, while all the other pages use the default layout.

@brillout
Copy link
Member Author

Labeling this as high-prio. I understand it's annoying, but I'd like to gauge how much and how often it's annoying in order to prioritize accordingly. The more I understand how annoying it is the more I will prioritize this.

@chrisvander
Copy link

chrisvander commented Oct 14, 2024

I have a layout with a sidebar and a right-hand panel that generally is centered with a maximum width, but occasionally needs to be broken out to take up the full right hand width. Currently, I would need to redefine the typical case on every page, and only occasionally leave out the layout so the child can take up the full width.

I bet there are other cases like this, but I wonder how the default and overrides should really work. There's definitely parts of the layout I want on every page, parts that I want on most pages, and parts that I want on some pages. There's a hierarchy there and I'm not sure whether it's defaults or overrides or some other way to configure the layout.

There are also cases where I want to swap out portions of the sidebar depending on the context, and not have that be dynamic, but just a part of those page's layout.

EDIT: This no longer matters to me. I set up a custom config flag that affects the layout. This works well for me.

@brillout
Copy link
Member Author

Indeed, using custom settings can be a workaround. Although note that all Layout code is loaded regardless of whether it's used then.

@brillout
Copy link
Member Author

Bumping to higest-prio (although it's the last prio among the highest prio).

I understand it's annoying, but I'd like to gauge how much and how often it's annoying in order to prioritize accordingly. The more I understand how annoying it is the more I will prioritize this.

Comment welcome. I could, maybe, implement this within a day.

@cr7pt0gr4ph7
Copy link

cr7pt0gr4ph7 commented Nov 7, 2024

(Edit: After digging through the source code and having a cold shower, I think there's a better & generic solution, see next comment)

Just hit upon this myself, and was quite suprised that there isn't an out-of-the box solution for it. After thinking about this a bit, here's my two cents on it - the problem is a bit more complicated than it appears on first thought.

I'm going to use the following hypothetical structure as an example:

(blank root)
  (root folder)
    login/
    admin/
    article/
      ratings/
      details/
      seller-statistics/
    cart/
      checkout/
        delivery/
        payment/

Some observations about the problem space:

  1. Pages form a hierarchy with respect to their folder paths. The main goal of this hierarchy is to divide a larger problem domain into logical chunks that appear well-reasoned to developers, so using folders is a good match.

  2. Pages form a second hierarchy with respect to their URLs, which is, by default, derived from their folder path. This hierarchy is also mostly developer-oriented, so reusing the first hierarchy by default is a good match.

  3. Pages form a third hierarchy with respect to how their layouts are nested. This third hierarchy often aligns with the first hierachy, but not always, as it's determined not by developer-centric considerations but by "What is a logical UI nesting structure from a user's perspective?".

    There are four cases I can think of:

    1. Fully replacing all parent layouts (e.g. for a fullscreen login/ page).
    2. Inserting a page "further up the layout tree" (e.g. a ratings/ page that does not reuse the rest of the article layout, but only the rest of the page layout shell as defined at (root folder)).
    3. Moving a page to a different location in the layout tree (e.g. moving seller-statistics/ to be a layout child of admin/).
    4. Replacing a part of the parent layout (like the sidebar example provided by @chrisvander)

A few sidenotes:

  • Layout replacements should probably also apply to child pages (e.g. a checkout/ flow with a minimalistic layout that is also applied to delivery/ and payment/).
  • "Fully replacing all parent layouts" can also be modeled as "making the page a child of the (blank root)", i.e. the root folder but without any custom defaults.
  • "Replacing a part of the parent layout" can be modeled as the parent layout providing customization slots as well as defaults for that slot using the meta setting. It might make sense to add some sample code that shows how to do this to the documentation, but otherwise I think this problem is solved.

Based on these considerations, I would propose something like this:

// +config.ts
// Alternative 1:
export default {
  Layout: {
    parent: "/some/other/folder" | "../" | null,
    use: LayoutForThisPageAndChildren
  }
}

// Alternative 2:
export default {
  Layout: {
    extends: "/some/other/folder" | "../" | null,
    use: LayoutForThisPageAndChildren
  }
}

// Alternative 3:
export default {
  extendsLayout: "/some/other/folder" | "../" | null,
  Layout: LayoutForThisPageAndChildren
}

Based on alternative 2, here's a more complete example:

// src/pages/+config.ts
export default {
  Layout: DefaultLayout
}

// src/pages/login/+config.ts
export default {
  Layout: {
    extends: null, // Use (blank root) as parent
    use: LoginLayout
  }
}

// src/pages/article/seller-statistics/+Layout.ts
export default {
  extends: "/admin",
  use: function ({ children }) {
    ...
  },
}

// src/pages/cart/checkout/+Layout.ts
export default {
  extends: "/(minimal-layout)",
  use: CheckoutLayout
}

// src/pages/(minimal-layout)/+Layout.ts
export default {
  extends: null,
  use: MinimalLayout
}

@cr7pt0gr4ph7
Copy link

cr7pt0gr4ph7 commented Nov 7, 2024

After digging through the source code of vike and giving it some time, I think there's a better & more generic solution that can be achieved by introducing an inherits config setting that controls which properties (accumuluated or not) are inherited from parent folders:

// src/pages/+config.ts
export default {
  Layout: DefaultLayout
};

// src/pages/login/+config.ts
export default {
  inherits: {
    Layout: false,
  },
  Layout: LoginLayout
};

// src/pages/article/seller-statistics/+config.ts
const AdminDefaults = {
  inherits: {
    Layout: false,
    title: false
  },
  Layout: [AdminLayout, DefaultLayout]
};

export default {
  extends: AdminDefaults
};


// src/pages/cart/checkout/+config.ts
import MinimalLayoutDefaults from 'pages/(minimal-layout)/defaults';

export default {
  extends: MinimalLayoutDefaults,
  Layout: CheckoutLayout
};

// src/pages/(minimal-layout)/+config.ts
export default {
  inherits: {
    Layout: false,
  },
  Layout: MinimalLayout
};

// src/pages/(minimal-layout)/defaults.ts
export default const MinimalLayoutDefaults = {
  inherits: {
    Layout: false
  },
  Layout: MinimalLayout
};

@drew-dulgar
Copy link

I am also interested in this inherited layout behavior. One thing that really caused some interesting behaviors is that my _errors/+Page.js file is also inheriting the default layout, which it shouldn't be. I had to basically move all of my base routes into a (app) folder to prevent errors from using the default layout.

I get the entire Vike stack in a bit of a recursive trace because if something errors out in the default layout, it calls the error page, which calls the default layout with an error again.

+1 for the inherits example above. I read over the initial suggestion and I'm not sure I quite understand the expected behavior with default/overrides option. It seems like it covers the needed scenarios, but it's not all that clear. I probably just need to mull it over.

@brillout
Copy link
Member Author

(I'll have a closer look at your proposal once I'm done with what I'm currently working on. ETA next week.)

@k9982874
Copy link

I really need this option. In my app, some subpages won't inherit the layout from the parent.
I can't implement this gracefully at the moment.

@alexmnv
Copy link

alexmnv commented Nov 17, 2024

@k9982874 For now, you can use a simple workaround: just define your own config setting for Layout rendering, which will not be cumulative.
Let's call it PageLayout:

meta: {
	PageLayout: {
		env: {server: true, client: true},
		cumulative: false,
	},
}

You can then render PageLayout inside the base layout. Here's an example using Vue:

<script setup lang="ts">
import {usePageContext} from 'vike-vue/usePageContext';

const pageContext = usePageContext();
const PageLayout = pageContext.config.PageLayout;
</script>

<template>
	<PageLayout>
		<slot />
	</PageLayout>
</template>

This approach allows you to define PageLayout separately for each page (without cumulation)

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

No branches or pull requests

8 participants