head icon indicating copy to clipboard operation
head copied to clipboard

support for merging `<head>` and `<body>` attrs

Open danielroe opened this issue 2 years ago • 3 comments

At the moment we can't merge classes, for example:

useHead({
  htmlAttrs: { class: 'first-class' }
})
useHead({
  htmlAttrs: { class: 'second-class' }
})

Sandbox

In this case there will be only one class applied to <html> rather than merging the two, which is what I would expect...

danielroe avatar Feb 18 '22 17:02 danielroe

If add merging, mb then an optional rewrite, for something like this?

// Case for rewrite color theme for some pages
useHead({
  htmlAttrs: { 
    class: {
      'foo': true,
      'theme-dark': true,
    },
  },
})
useHead({
  htmlAttrs: { 
    class: {
      bar: true
      'theme-dark': false,
      'theme-light': true,
    },
  },
})

// result: `foo bar theme-light`

Kolobok12309 avatar Aug 05 '22 10:08 Kolobok12309

Hey @danielroe

If this is still something you're interested in, do you have any ideas on implementation?

Merging being the default behaviour may cause some breaking changes, if we add a way to opt-in to merging, I'm not too sure what that would look like.

Would adding a new function such as useHtmlAttrs be useful?

Otherwise can potentially add this as a breaking change for 1.x

harlan-zw avatar Sep 19 '22 05:09 harlan-zw

Definitely would be useful. I imagine there are some edge cases where it's not 100% clear what to do, but if we implement in the most naive way it would probably work for most users, and if it's opt-in (or could be opt-out) then it should be fine.

Implementing via a new useHtmlAttrs seems like a good idea to not make it breaking.

theoephraim avatar Oct 07 '22 04:10 theoephraim

We're also facing this issue on having a case to merge the attrs. Anternative approach could be allow something like the HandlesDuplicates interface as an option? Then each property could have a 'marge' attribute indicating if the value should be merged or not? This would provided the possibility to both define merge or overwrite.

interface HandlesAttrsMerge {
  /**
   * Indicates if the attribute should merge into an existing property
   */
  merge?: boolean,
  value: string
}

tkjaergaard avatar Oct 28 '22 10:10 tkjaergaard

Thanks for the suggestion @tkjaergaard

In the next version you'll be able to modify the duplicate strategy, by default it's replace, but you'll be able to change it to merge.

harlan-zw avatar Oct 30 '22 07:10 harlan-zw

@harlan-zw Nice, is for the individual attrs or du you have to eiher replace or merge all?

For example, let say you got:

useHead({
  htmlAttrs: {
     class: {
         'foo': true
     },
     'custom-attr': 'bar'
  }
})

now i want to replace only the class attribute:

useHead({
  htmlAttrs: {
     class: {
         'bar': true
     }
  }
})

Can we then still define that's it's only the class attribute that we want to replace? 🙂

tkjaergaard avatar Nov 04 '22 09:11 tkjaergaard

I've given this one some thought and I agree with Daniel's original point, that merging is intuitive behavior.

It's also intuitive that classes should work like Vue, with array and class syntax support.

The following will be supported soon:

Merging by default

useHead({
  htmlAttrs: { class: 'first-class' }
})
useHead({
  htmlAttrs: { class: 'second-class' }
})

// <html class="first-class second-class">

Object class support, including refs

const theme: Ref<'dark' | 'light'> = ref('dark')

useHead({
  htmlAttrs: {
    class: {
      'layout-theme-dark': () => theme.value === 'dark',
      'layout-theme-light': () => theme.value === 'light',
    },
  },
  bodyAttrs: {
    class: ['test', () => `theme-${theme.value}`]
  }
})

const page: Ref<{ name: string }> = ref({ name: 'home' })
useHead({
  htmlAttrs: {
    class: () => page.value.name
  },
})

// <html class="layout-theme-dark home">
// <body class="test theme-dark">

If you need to replace the attributes then you can use the property

useHead({
  htmlAttrs: { class: 'first-class' }
})
useHead({
  htmlAttrs: { class: 'second-class', tagDuplicateStrategy: 'replace' }
})

// <html class="second-class">

Let me know if you have any further feedback.

harlan-zw avatar Nov 06 '22 12:11 harlan-zw

@harlan-zw The main issue that i see, is that you either merge all or nothing?

Or would you call useHead multiple times depending on the strategy?

useHead({
  htmlAttrs: {
    class: ["foo", "bar"],
    "data-theme": "dark"
  }
})

// <html class="foo bar" data-theme="dark">
useHead({
  htmlAttrs: {
    class: ["taz"],
    tagDuplicateStrategy: "replace"
  }
})

// <html class="taz" data-theme="dark">

useHead({
  htmlAttrs: {
    "data-theme": "alternate"
  }
})

// <html class="taz" data-theme="dark alternate">

Or how is expected behavior? 😊

tkjaergaard avatar Nov 06 '22 16:11 tkjaergaard

I don't see a real use case for not merging everything. Would you mind providing a real-life example?

For the theme example these changes would presumably happen in a root-level component (app.vue, layouts/default.vue), so doesn't seem reasonable to change the theme from a child-component (better to have a global ref storing the theme).

This is merging at a hierarchical level, if you had separate classes for different pages, they would be replaced.

There is an escape hatch if you do need finer control over the merging logic as there will be hooks available

harlan-zw avatar Nov 06 '22 22:11 harlan-zw

@harlan-zw We have cases where child-components sets html/body attrs based on visibility. Let's say you have some sort of sticky navigation that changes color based on a background (defined in in a child component).

But if the tagDuplicateStrategy only affects the properties defined in the call, i think that could work fine.

But can you confirm that my previous example is how it would work? or would replace strategy replace all attributes? 🙂

tkjaergaard avatar Nov 07 '22 11:11 tkjaergaard

Hey @tkjaergaard

You're welcome to play around with the latest next version and if you notice behaviour that isn't expected let me know and we can figure out a solution.

harlan-zw avatar Nov 11 '22 09:11 harlan-zw

This is now in the v1 stable release, attribute props will be merged by default.

You can find documentation here: https://unhead.harlanzw.com/guide/guides/handling-duplicates#tagduplicatestrategy

harlan-zw avatar Nov 14 '22 06:11 harlan-zw