-
Notifications
You must be signed in to change notification settings - Fork 9
Display blogs in terslated language #10
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
base: main
Are you sure you want to change the base?
Conversation
…et français pour une traduction
…gue dans un post nous envoie automatiquement sursa version traduite
…as bien donc faut remodifier derrière en changeant la db mais bon au moins j'ai pas tout à refaire
…permettant d'alterner d'une page à sa page traduite. Tout ça en adaptant les ajouts fait au préalable pour la db précédente
…réation d'un post + tentative de modifier le bouton de langue pour swap les posts mais marche pas donc je fais une redirection sur les pages du blog en changeant la langue
t-webber
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for this PR!
A few issues need to be address in terms of code, I left you a review to that extent
| import { isLocale } from "@/locales/config"; | ||
|
|
||
| function Rename({ title, id, router }: { title: string; id: number; router: AppRouterInstance }) { | ||
| function Rename({ title, id, router, t, locale }: { title: string; id: number; router: AppRouterInstance; t: any; locale: Locale }) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't use any, if you to type a part of the dictionary, use the Dictionary type (e.g; if you wanted to get the title of the signin page, you should write t: Dictionary.navigation.cookies.signin.title or t: Dictionary["navigation"]["cookies"]["signin"]["title"])
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
| } | ||
|
|
||
| model Post { | ||
| id Int @id @unique @default(autoincrement()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use the prisma extension + formatOnSave.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
| } | ||
|
|
||
| function AddLabel({ getLabels, addRemoveLabel, dbLabels }: { getLabels: string[]; addRemoveLabel: (x: string) => void; dbLabels: string[] }) { | ||
| function AddLabel({ getLabels, addRemoveLabel, dbLabels, t }: { getLabels: string[]; addRemoveLabel: (x: string) => void; dbLabels: string[]; t: any }) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as before, don't use any
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
| } | ||
|
|
||
| function OpenSave({ saving }: { saving: boolean }) { | ||
| function OpenSave({ saving, t }: { saving: boolean; t: any }) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
| } | ||
|
|
||
| export function Actions({ setToBeChanged, content, value, title, id, dbLabels, blogLabels, locale }: ActionProps) { | ||
| const localepage = usePathname().split("/")[1] as Locale; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Never do this. You should NEVER use usePathname. Please use Next.js dynamic routing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed using the locale from the page directly in page.tsx
| <div className="p-10 h-full"> | ||
| <DataTable | ||
| search_column="title" | ||
| search_column={"title"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this modification ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess I used a variable then changed it back but forgto the brackets. But yeah it's useless
| import { Checkbox } from "@/components/ui/checkbox"; | ||
| import Link from "next/link"; | ||
| import { usePathname } from "next/navigation"; | ||
| import { Locale } from "@/locales/config"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unused imports,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
| redirect(nav(locale, "/error/404")); | ||
| } | ||
| const localePost = posts.find(blog => blog.slug === slug); | ||
| const localePost = posts.find(blog => blog.slug === slug || blog.slugtr === slug); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This means that if I translate a blog, it wont have priority ?
I thing you need to find the blogs with the right slug, and then try slugtr if you haven't found anything
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Posts have a locale attribute. If one has "fr" then the translated one has "en". So by searching for blogs through the locale you can only get one of them. And as each slug and slugtr are unique you are guaranted to find the correct one because you search using postId which is slug of the post you are willing to translate. The locale + slug makes it so that you exactly know if your post is the original one or the translated one.
There's no priority issue as you can only get either one of them.
Or maybe I didn't understand what you meant ?
src/db/blogs.ts
Outdated
| await db.post.update({ | ||
| where: { id: id }, | ||
| data: { title }, | ||
| data: { title: title }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why? this change seems useless
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed, I might have added something else and changed it again I don't remember. Maybe from when I had added titlefr and titleen in the db before I thought of slugtr
…tent en deux langues, cela peut créer des duplications.
t-webber
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few comments concerning the code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
choose better name, like "add-slugtr.ts"
We already know it concerns the db as it is inside the prisma folder 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done !
|
Please also insure your code is formatted (having formatOnSave + prettier is recommended) |
(I don't see where but it tells me so whatever)
|
any update on this PR @ImTooFastForYou ? |
No, unless you have anything else to say it should be done. Last time I checked, my code updated the prisma model correctly and the migration went good. It's just a matter of doing the manipulations correctly on your side ig |
|
Okay, the changes were still written as requested; don't forget to mark them as done |
J'ai principalement focus sur la gestion de l'anglais pour les posts du blog.
-Lors de la création d'un post, la langue du post peut être sélectionnée. Il pourrait être intéressant de ne mettre aucune langue par défaut et afficher un texte pour rappeler à l'auteur de choisir sa langue pour éviter des problèmes par la suite.
-Pour cela, la création d'un post entraine la création d'un post jumeau dans l'autre langue dont le titre est celui du post original + non_defined.
-J'ai modifié la db en ajoutant un slugtr de sorte à établir un lien entre le post et son post jumeau, j'ai aussi modifié le type PostPresentation de la même manière.
-Ce slugtr permet de fixer le problème qui avait lieu quand on voulait changer de langue en étant sur la page d'un post. Désormais changer la langue entrainera une redirection vers le post jumeau (à condition que ce dernier ait été validé, on pourrait rajouter une option pour gérer autrement si le jumeau n'a pas été validé).
-J'ai rajouté des traductions en anglais à quelques pages comme celle de la création de post ou les boutons disponibles quand on est connecté avec les droits, j'ai donc modifié le dictionnaire pour ça.
Je crois que c'est tout pour les ajouts, si y'en a d'autres je m'en souviens plus. J'ai vu une erreur sur l'affichage des noms des auteurs sur la page du blog. De ce que j'ai pu tester, les noms ne sont pas ajoutés à la db pour je ne sais quelle raison. Le bug existe déjà sur le site néanmoins quand on change la langue il semble que ça casse donc je ne sais pas si j'en suis totalement responsable (parce que moi ça s'affiche pas tout court) mais je préfère le signaler. Aussi le bouton pour changer de langue n'affiche plus la bonne langue mais ça c'est pas moi c'est une des autres PR mais pareil je le signale.