-
-
Notifications
You must be signed in to change notification settings - Fork 22
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: improve translations editing #324
Conversation
…nction update_nodes
…zontally, refactor update_nodes
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 PR, does what it is supposed to do!
However, I left quite a bit of comments concerning the previous code's quality
Not sure if it should be handled in this PR or not... (But my stance on that is to refactor at the same time of adding new features, or better yet to do a refactor PR before the feat changes)
taxonomy-editor-frontend/src/pages/editentry/AccumulateAllComponents.jsx
Show resolved
Hide resolved
taxonomy-editor-frontend/src/pages/editentry/AccumulateAllComponents.jsx
Show resolved
Hide resolved
taxonomy-editor-frontend/src/pages/editentry/ListTranslations.tsx
Outdated
Show resolved
Hide resolved
taxonomy-editor-frontend/src/pages/editentry/ListTranslations.tsx
Outdated
Show resolved
Hide resolved
taxonomy-editor-frontend/src/pages/editentry/ListTranslations.tsx
Outdated
Show resolved
Hide resolved
taxonomy-editor-frontend/src/pages/editentry/ListTranslations.tsx
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.
I see @eric-nguyen-cs did a good review, I didn't take the time to understand tsx files much, sorry.
taxonomy-editor-frontend/src/pages/editentry/AccumulateAllComponents.jsx
Show resolved
Hide resolved
ef4d88e
to
6e3df23
Compare
6e3df23
to
02b450b
Compare
@eric-nguyen-cs I let you validate it if @perierc thinks it's ready. |
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.
You can merge after solving the important comments
What
This PR changes a few things about the edit entry page:
This PR also includes a lot of refactoring to simplify the code and avoid unnecessary rerenders.
Video demo
new-node-edition.mp4
ID change (first translation of the main language changes) :
id-change.mp4
Part of
#323