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.

Sign up for GitHub

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

Jump to bottom

fix: locale head reactivity on locale change for strategy: 'no_prefix' #2897

Merged
merged 3 commits into from Apr 4, 2024
Merged

fix: locale head reactivity on locale change for strategy: 'no_prefix' #2897

merged 3 commits into from Apr 4, 2024

Conversation

BobbieGoede
Copy link
Collaborator

🔗 Linked issue

❓ Type of change

  • 📖 Documentation (updates to the documentation, readme or JSdoc annotations)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • 👌 Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

Resolves #2871

Even though SEO tags don't make much sense for the unprefixed routing strategy, the locale head composable also handles the lang and dir attributes which do need to be updated.

📝 Checklist

  • I have linked an issue or discussion.
  • I have added tests (if possible).
  • I have updated the documentation accordingly.

@BobbieGoede BobbieGoede requested a review from kazupon April 3, 2024 12:41
@BobbieGoede BobbieGoede self-assigned this Apr 3, 2024
@kazupon
Copy link
Collaborator

kazupon commented Apr 3, 2024

This PR has still regression, that implementation still looks not enough to me.
https://github.com/nuxt-modules/i18n/actions/runs/8538876258/job/23392425785

@BobbieGoede
Copy link
Collaborator Author

This PR has still regression, that implementation still looks not enough to me. nuxt-modules/i18n/actions/runs/8538876258/job/23392425785

Normally we wait for URL change to deal with flakiness but that's not possible for no_prefix so this made the test flaky, I added a workaround for this.

kazupon
kazupon approved these changes Apr 4, 2024
View reviewed changes
Copy link
Collaborator

@kazupon kazupon left a comment

Choose a reason for hiding this comment

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

It looks okay!

@kazupon kazupon merged commit 8961513 into nuxt-modules:main Apr 4, 2024
7 checks passed
DarthGigi pushed a commit to DarthGigi/i18n that referenced this pull request Apr 16, 2024
@BobbieGoede
fix: locale head reactivity on locale change for `strategy: 'no_prefi…
1b36982 
…x'` (nuxt-modules#2897)

* fix: locale head not updating on locale change using `strategy: 'no_prefix'`

* test: add locale head tests for `'no_prefix'` strategy

* test: fix flakiness by waiting for title change
@meirroth
Copy link

@kazupon When will this fix be released?

@BobbieGoede
Copy link
Collaborator Author

@meirroth
I just published v8.3.1 that includes this change

@nuxt-modules nuxt-modules locked as resolved and limited conversation to collaborators Apr 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@kazupon kazupon kazupon approved these changes

Assignees

@BobbieGoede BobbieGoede

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

SEO attributes are not reactive (lang, dir html attribute) with no prefix strategy
3 participants
@BobbieGoede @kazupon @meirroth