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

Replace withTranslation HOC with hooks and test against actual i18n'd values #4017

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Replace withTranslation HOC with hooks and test against actual i18n'd values #4017

wants to merge 1 commit into from

Conversation

cbeer
Copy link
Collaborator

@cbeer cbeer commented Dec 5, 2024

No description provided.

@cbeer cbeer force-pushed the useTranslation-hook branch from 16a5e3b to eccc503 Compare December 5, 2024 18:30
@cbeer cbeer marked this pull request as ready for review December 5, 2024 23:11
@cbeer cbeer changed the title Use translation hook Replace withTranslation HOC with hooks. Dec 5, 2024
@marlo-longley
Copy link
Member

This is awesome so much cleaner, thank you Chris !!

  1. With Update search components to functions. #3999 merged you can rebase the Search components which will limit their changes just to the translation stuff (and not the functional conversion)
  2. I guess we will need to rewrite the Jest stuff for Vitest but it doesn't look that bad.

@cbeer cbeer force-pushed the useTranslation-hook branch from eccc503 to 5079b84 Compare December 6, 2024 16:39
@cbeer
Copy link
Collaborator Author

cbeer commented Dec 6, 2024

Yes, I think those are the next steps, and I'm hopeful this'll merge nicely into the vite branch (and if it doesn't, it should just be removing lines of code...)

@cbeer cbeer force-pushed the useTranslation-hook branch 3 times, most recently from ce98664 to 3be0e1e Compare December 6, 2024 18:45
@marlo-longley
Copy link
Member

Needs rebase but looks good to me.

@cbeer cbeer force-pushed the useTranslation-hook branch 2 times, most recently from 26ede06 to 7124e2a Compare December 10, 2024 16:11
@cbeer cbeer changed the title Replace withTranslation HOC with hooks. Replace withTranslation HOC with hooks and test against actual i18n'd values Dec 10, 2024
@cbeer cbeer force-pushed the useTranslation-hook branch from 7124e2a to 4199d2c Compare December 10, 2024 16:31
marlo-longley
marlo-longley previously approved these changes Dec 10, 2024
View reviewed changes
@cbeer cbeer force-pushed the useTranslation-hook branch from 4199d2c to a6d0917 Compare December 11, 2024 15:48
@cbeer cbeer force-pushed the useTranslation-hook branch from a6d0917 to cf28d6e Compare December 11, 2024 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marlo-longley marlo-longley marlo-longley left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@cbeer @marlo-longley