-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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(relation-widget): i18n options #6299
fix(relation-widget): i18n options #6299
Conversation
@demshy Can you give more details in your description about how to set up the config file so that one can get the same result as your second screenshot? To be more specific, what goes under the Travel Idea Type collection and what the relation widget setup looks like? |
Hi, of course, I completely forgot about this. I resolved this an extension of another issue (#6264) which can be closed so I used the test case from that issue. Anyhow, I have added the collection config and example json data file to the description. |
Yes, this was the idea. Currently english texts would be displayed on both sides. |
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.
Good job in retrieving options in the correct locale. However, the locale
prop is being passed through several intermediary components that don't explicitly use them, namely EditorControlPane
and EditorControl
.
What do you think about using the context API to let Widget
subscribe to locale
value from EditorControlPane
?
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 @demshy, this works great and sorry for the very late review.
@bytrangle great point on using context API. I think this PR is consistent with the rest of the codebase, so I'm happy to ship it as is to fix the issue. We can re-visit the usage of context API in the codebase as a general approach, WDYT?
We can re-visit context API in a separate PR
Hey guys, sorry for my late response, but this was not a blocker for us and I had to attend to other matters for a while. |
@erezrokah I'm fine with your suggestion. |
@demshy Do you mind if I work on the context API? I'm already working on it locally and it's still fresh in my mind. It's alright if you want to work on it. |
@bytrangle of course, that would be great! I will try to delay our thing until you're done. |
Was this fix deployed to a release? I'm running |
Summary
Options in relation widget were not translated for collection or file based relations. I passed the
locale
into the component so that the function that parses these options can use it accordingly.fixes #4491
Test plan
As I'm new here, I didn't feel quite comfortable adding props to the main editor blocks, but didn't find another way to access locale of the component that is rendered. If there is another (better) way, please inform me and I will update this accordingly.
Config:
content in data/travelTypes.json
Checklist
yarn format
.yarn test
.A picture of a cute animal (not mandatory but encouraged)
![a69429a0f5a4e808171d1be9751b6a83--cute-baby-animals-farm-animals](https://user-images.githubusercontent.com/19568230/158430697-1cc87267-c2ff-4b00-bd85-1060abde5037.jpg)