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

fix: stabilize t in app directory #1206

Merged
merged 1 commit into from
Apr 9, 2024
Merged

Conversation

jessemartin
Copy link

What is the purpose of this pull request? (put an "X" next to item)

[ ] Documentation update
[x] Bug fix
[ ] New feature
[ ] Other, please explain:

What changes did you make? (Give an overview)
stabilized t in app directory

Which issue (if any) does this pull request address?
#1205

Is there anything you'd like reviewers to focus on?

@jessemartin
Copy link
Author

@aralroca mind taking a look?

Copy link
Owner

@aralroca aralroca left a comment

Choose a reason for hiding this comment

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

This PR remember me this other PR: #574 that was reverted here because introduce this issue #604

can you verify that now shared layouts work with your change?

@jessemartin jessemartin force-pushed the canary branch 4 times, most recently from a851d30 to 3e96897 Compare April 2, 2024 00:02
@jessemartin
Copy link
Author

This PR does not change the behavior of the hook in the Pages router, so no regression is present there. I tested out shared layouts in the App router via the with-app-directory and it seems to be working as intended!

demo.mov

src/useTranslation.tsx Outdated Show resolved Hide resolved
@jessemartin
Copy link
Author

@aralroca could you please have another look at this?

Copy link
Owner

@aralroca aralroca left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution

@aralroca
Copy link
Owner

aralroca commented Apr 8, 2024

@jessemartin I have updated the canary branch because in master there were changes that were not in the canary, and just failed the test "t is stable", could you check what happened? Thanks and sorry for the inconvenience

@jessemartin
Copy link
Author

@aralroca looks like the problem was a merge conflict (the upstream logic had moved to createTranslation.tsx). I force-pushed a rebase and fix to this branch.

@aralroca aralroca merged commit 4085539 into aralroca:canary Apr 9, 2024
6 checks passed
@aralroca
Copy link
Owner

aralroca commented Apr 9, 2024

Thanks a lot @jessemartin 😊

@aralroca
Copy link
Owner

aralroca commented Apr 9, 2024

@all-contributors please add @jessemartin for code

Copy link
Contributor

@aralroca

I've put up a pull request to add @jessemartin! 🎉

@kristian240
Copy link

@aralroca can you create a new canary release that ships this?

@aralroca
Copy link
Owner

aralroca commented Oct 25, 2024

@kristian240 I've pre-released 3.0.0-canary.4 now with the latest changes in the canary branch. Give feedback if everything is ok. I have to make revision and re-take the route-map for 3.0.0.

I feel that in the last months there has been a lot of focus on Brisa framework, now it's time to work for 3.0.0 version of next-translate and give good support on Next.js 15.

@kristian240
Copy link

This was quick, thanks a lot.

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

3 participants