-
-
Notifications
You must be signed in to change notification settings - Fork 23
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
feat: List all properties of an entry #45
Conversation
taxonomy-editor-frontend/src/pages/editentry/ListAllProperties.jsx
Outdated
Show resolved
Hide resolved
taxonomy-editor-frontend/src/pages/editentry/ListAllProperties.jsx
Outdated
Show resolved
Hide resolved
taxonomy-editor-frontend/src/pages/editentry/ListAllProperties.jsx
Outdated
Show resolved
Hide resolved
taxonomy-editor-frontend/src/pages/editentry/ListAllProperties.jsx
Outdated
Show resolved
Hide resolved
if (key.startsWith('prop')) { | ||
|
||
// Removing "prop_" prefix from key to render only the name | ||
const property_name = key.split('_').slice(1).join('_'); |
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.
what about doing this property_name extraction only when the property name is shown? I mean, inside the <Typography sx={{...}}>{property}:</Typography>
. The renderedProperties
doesn't seems to care if it has or not the preffix prop_
, and the issue is that in the onChange
you need to add it again
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.
@nobeeakon renderedProperties
does care about the prefix prop_
. The key-value pairs that have to be rendered on the webpage should have the prefix prop_
. Hence, the appropriate checks have been done in the start.
taxonomy-editor-frontend/src/pages/editentry/ListAllProperties.jsx
Outdated
Show resolved
Hide resolved
taxonomy-editor-frontend/src/pages/editentry/ListAllProperties.jsx
Outdated
Show resolved
Hide resolved
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.
nice work 🚀
What