-
Notifications
You must be signed in to change notification settings - Fork 5
[#3833]: Content - Item with relational field not reflecting correct locale when switch to other language version #3871
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: dev
Are you sure you want to change the base?
Conversation
| const [prevFirstContentFieldValue, setPrevFirstContentFieldValue] = | ||
| useState(null); | ||
| const languages = useSelector((state) => state.languages); | ||
| const contentItems = useSelector((state) => state.content); |
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 is a pretty significant selector that will cause the whole Editor to re-render on content change. We should move it as low as possible in the component tree
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.
removed contentItems selector
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.
.
| <div className={styles.Fields}> | ||
| {activeFields?.map((field) => { | ||
| const fieldItem = contentItems?.[item?.data?.[field.name]]; | ||
| const fieldValue = ["one_to_one", "one_to_many"].includes( |
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.
Related to previous comment. Would it not be a better idea to add this business logic in the relationship field component itself specifically in these types case blocks? since it only relates to specific field types and is much lower in the component tree
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.
Agreed. That's a much better approach. I've moved the locale-update logic into the relationship field component as suggested.
| @@ -1,5 +1,5 @@ | |||
| import { memo, useCallback, useEffect, useMemo, useState } from "react"; | |||
| import { useDispatch } from "react-redux"; | |||
| import { useDispatch, useSelector } from "react-redux"; | |||
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 import
| ]); | ||
|
|
||
| useEffect(() => { | ||
| updateItemLocale(); |
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 seems like a very hacky solution to this. If im reading this correctly you are changing the content's state and then manually unmarking it as dirty on mount? Are we also changing values? If so why? We should not be doing any kind of content change that is not explicit by the user
A more holistic solution is when switching locale the value of the initial state should reflect the 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.
The content of all locale items will be identical, as they inherit the same content upon creation.
I’m not sure if we have control over that behavior on the frontend.
|
Based on discussions @geodem127 we should close or mark the pr as draft. Don't want this accidentally merged in |
Root Cause:
The relational field inherits the language selection from when the source item was originally created.
Fix:
Implemented logic to retrieve content items localized to the currently selected language.
Loom.Message.-.22.October.2025.1.mp4