-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add check for translation support #4
Conversation
src/components/ProjectContents.jsx
Outdated
<p>Title: { project_contents.title }</p> | ||
<p>Description: { project_contents.description }</p> | ||
<p>Introduction: { project_contents.introduction }</p> | ||
<p data-translation-project-contents={project_contents}>Title: { project_contents.title }</p> |
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.
HTML attributes are strings, so this generates data-translation-project-contents="[object Object]"
.
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.
Mm I'll have to rethink how to get the resource's data to the container then.
src/helpers/isElementTranslatable.js
Outdated
@@ -0,0 +1,8 @@ | |||
function isElementTranslatable(event) { | |||
if (event.target.dataset.translationProjectContents) { |
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'm not sure what this does if event.target.dataset
is null or undefined.
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.
Ack, it'd crash, thanks.
this.setState({ | ||
modalOpen: true, | ||
projectContents: { | ||
description: projectContentsData.description, |
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.
Is this just copying projectContentsData
to projectContents
? If so, why not rename projectContentsData
to projectContents
?
translationText: event.target.textContent, | ||
}); | ||
} else { | ||
return alert('This element does not support translations'); |
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.
return
is not needed here.
@eatyourgreens are you happy with this? |
<p>Title: { project_contents.title }</p> | ||
<p>Description: { project_contents.description }</p> | ||
<p>Introduction: { project_contents.introduction }</p> | ||
<p data-translation-project-contents={project_contents.title}>Title: { project_contents.title }</p> |
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.
Should these data attributes be storing the names of the keys, rather than their values? Otherwise, I'm not seeing how the values get mapped back to keys when they're edited.
This is probably outdated now, given that most things have been generalised. I'll wait to close until I've wired up the frontend with create/update functions |
Superseded. |
Based on custom
data-
attributes.