Skip to content

feat: replace useHead with ionic-compatible head implementation - #518

Merged
danielroe merged 17 commits into
nuxt-modules:mainfrom
chibx:main
May 22, 2024
Merged

feat: replace useHead with ionic-compatible head implementation#518
danielroe merged 17 commits into
nuxt-modules:mainfrom
chibx:main

Conversation

@chibx

@chibxchibx commented Feb 22, 2024

Copy link
Copy Markdown
Contributor

I created an utility function, useIonHead, that works almost the same as useHead from unhead, but just with a hack around.
The function provides a way to dispose and patch the created head object just like useHead.
During the process, I realized that tabbed view was a big problem when trying to create this util, and the randomness of the Ionic Hooks. The main issue was leaving from a tabbed view to a non-tabbed view and trying to mantain or destroy state.
Guess What: You can nest useIonHead calls within children components and still retain the same structure after the page it was created on, is brought into view, and it's also fast as well.

resolves#6

I figured out that the author of @vueuse/head, @egoist, suggested that developers should go for the unhead package, and so I decided to create an utility function with unhead, to handle <head></head> tags management accurately, irrespective of the Ionic hooks randomness.
Note: There is also support for nested head calls within components
@netlify

netlifyBot commented Feb 22, 2024

Copy link
Copy Markdown

Deploy Preview for friendly-lamington-fb5690 canceled.

NameLink
🔨 Latest commitede24be
🔍 Latest deploy loghttps://app.netlify.com/sites/friendly-lamington-fb5690/deploys/664e646f34e8000008ed7040

@danielroe

Copy link
Copy Markdown
Collaborator

This is fantastic! Do you think we should overwrite useHead instead of creating an additional composable?

@chibx

chibx commented Mar 26, 2024

Copy link
Copy Markdown
ContributorAuthor

This is fantastic! Do you think we should overwrite useHead instead of creating an additional composable?

@danielroe Sure, I guess changing the name to useHead would be better for ease when working with ongoing projects

@danielroe
danielroe self-requested a review as a code ownerMay 19, 2024 20:11
@codecov

codecovBot commented May 20, 2024

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 3.73832% with 103 lines in your changes are missing coverage. Please review.

Project coverage is 46.65%. Comparing base (f8e27aa) to head (ede24be).
Report is 2 commits behind head on main.

FilesPatch %Lines
src/runtime/composables/head.ts0.00%99 Missing ⚠️
src/module.ts57.14%3 Missing ⚠️
src/runtime/plugins/router.ts0.00%1 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #518 +/- ##
==========================================
- Coverage 52.36% 46.65% -5.72% 
==========================================
Files 10 11 +1 Lines 762 866 +104 Branches 37 38 +1 ==========================================
+ Hits 399 404 +5 - Misses 363 462 +99 

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@danielroedanielroe changed the title fix for @vueuse/head doesn't function properly with ionicfeat: add useIonHead for ionic-compatible head metadataMay 20, 2024

@danielroedanielroe left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the next step would be to write a browser test that asserts the correct title after client navigation, following the pattern here:

https://github.com/danielroe/nuxt-ionic/blob/a45c95693f1fdb8de0be43d7a664db72989effd2/test/e2e/ssr.spec.ts#L27-L36

Is that something you would be happy & able to do?

@chibx

Copy link
Copy Markdown
ContributorAuthor

I think the next step would be to write a browser test that asserts the correct title after client navigation, following the pattern here:

https://github.com/danielroe/nuxt-ionic/blob/a45c95693f1fdb8de0be43d7a664db72989effd2/test/e2e/ssr.spec.ts#L27-L36

Is that something you would be happy & able to do?

@danielroe I will be able to do this

@chibx

Copy link
Copy Markdown
ContributorAuthor

@danielroe Already created a new test in the e2e/ion-head.spec.ts file, and also changed useIonHead to useHead with a higher priority.

Made a few more changes to the tests due to timeout and memory consumption from previously ongoing tests, ssr.spec.ts, by closing the open page, shaving off a huge amount of allocated memory.

I added delays after each toggle of the button in the test to represent the delay that useHead takes to rectify changes to the document's head tag, but this brought about a reason to increase the timeout for the test.

I hope the test satisfies the required condition

@danielroedanielroe mentioned this pull request May 22, 2024
@danielroedanielroe changed the title feat: add useIonHead for ionic-compatible head metadatafeat: replace useHead with ionic-compatible head implementationMay 22, 2024

@danielroedanielroe left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much for this ❤️

@danielroe

Copy link
Copy Markdown
Collaborator

cc: @harlan-zw - you may have a better idea or improvements to suggest

@danielroe
danielroe merged commit b9a4f97 into nuxt-modules:mainMay 22, 2024

@chibxchibx left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danielroe Thanks for correcting the test. This obviously looks better.

@harlan-zw

Copy link
Copy Markdown

I'm not entirely sure I understand the issue, the Ionic lifecycle hooks don't align with the Vue lifecycle hooks?

If you have any capacity to make an issue on the Unhead repo with some technical background I'd be happy to explore some alternative ways of solving it that don't rely on new composables.

This solution seems quite good though, nice job!

@atakansn

Copy link
Copy Markdown

hello, I can not useHead, what should I do?

@chibx

Copy link
Copy Markdown
ContributorAuthor

@atakansn What version of the useHead package are you currently on

@atakansn

Copy link
Copy Markdown

@chibx @unhead/vue version 2.0.0-rc.8 I use nuxt 3 and @nuxtjs/ionic side

@chibx

chibx commented Mar 18, 2025

Copy link
Copy Markdown
ContributorAuthor

@chibx @unhead/vue version 2.0.0-rc.8 I use nuxt 3 and @nuxtjs/ionic side

I suppose this is why
The next question would be if you are importing directly from unhead/vue or from #imports, and if not from #imports, please try that
If the issues persist, try setting the head.legacy key in your Nuxt config to true

I'll look into this

@atakansn

Copy link
Copy Markdown

@chibx When I import { useHead } from '@unhead/vue' or import { useHead } from '#app', useHead works.

@chibx

Copy link
Copy Markdown
ContributorAuthor

@chibx When I import { useHead } from '@unhead/vue' or import { useHead } from '#app', useHead works.

So now does it work as expected?
I am asking this because before there was an open issue regarding the native useHead function not working in an Ionic environment because Ionic doesn't just simply destroy the DOM, but instead hides it, so useHead had no way to know when the component was back in focus

So I created a stub that should be working in @nuxtjs/Ionic, and it should be registered as auto-import from the module itself

@atakansn

Copy link
Copy Markdown
useHead({
title: 'Title',
htmlAttrs: {
class: 'h-full bg-gray-100'
}, meta: [
{ name: 'description', content: 'My amazing site.' }
],
bodyAttrs: {
class: 'test'
},
script: [ { innerHTML: 'console.log(\'Hello world\')' } ]
})

in the example it adds all properties

Sign up for freeto 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.

fix: @vueuse/head doesn't function properly with ionic

5 participants

@chibx@danielroe@harlan-zw@atakansn@Parkoba