Skip to content
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

Rte translate button + translation via xml transformation #1484

Conversation

VP-DS
Copy link
Contributor

@VP-DS VP-DS commented Dec 5, 2023

#1587 New Pull Request using comet repo branch instead of forked one

@VP-DS
Copy link
Contributor Author

VP-DS commented Dec 5, 2023

There is no good way to disable the translation button like for the FinalFormInput with adding a hideTranslate props.
There is SupportedThings on the rte but that naming would not match an hideTranslate option.

Or does this not matter here?

@VP-DS VP-DS force-pushed the rte-translate-button branch 2 times, most recently from 216df08 to 799c878 Compare December 5, 2023 13:30
@VP-DS VP-DS mentioned this pull request Dec 6, 2023
@VP-DS VP-DS marked this pull request as draft December 6, 2023 15:07
@VP-DS VP-DS self-assigned this Dec 11, 2023
@VP-DS VP-DS marked this pull request as ready for review December 11, 2023 08:48
@nsams
Copy link
Member

nsams commented Dec 11, 2023

is this xml transformation really necessary? that is very complaex and possibly incomplete

@VP-DS
Copy link
Contributor Author

VP-DS commented Dec 11, 2023

is this xml transformation really necessary? that is very complaex and possibly incomplete

It is necessary to keep the knowledge of which words have inline stylings after the translation.

(The xml code was copied from https://github.com/vivid-planet/comet/pull/1040/files)

I tried some draftJSToHtml + htmlToDraftJS package as an alternative, but there you lost subscript and superscript. (https://gitlab.vivid-planet.com/vivid/groupcms-service/-/merge_requests/847/diffs#fd8a3767a51ea9ca2845f07f7dfaaa75bac6683c_8_8 maybe i could use it with the customInlineFn 🤔 )

@VP-DS VP-DS force-pushed the rte-translate-button branch 3 times, most recently from 6bbe72d to b16cb28 Compare December 18, 2023 12:00
@VP-DS VP-DS force-pushed the rte-translate-button branch 5 times, most recently from 78c68fb to b60c705 Compare December 20, 2023 10:10
@VP-DS VP-DS force-pushed the rte-translate-button branch 2 times, most recently from f4b2683 to b60c705 Compare January 3, 2024 14:42
@VP-DS VP-DS changed the base branch from feature/translation-module to feature/translation-module2 January 5, 2024 06:42
@VP-DS VP-DS changed the base branch from feature/translation-module2 to feature/translation-module January 5, 2024 06:43
@VP-DS VP-DS marked this pull request as draft January 11, 2024 07:38
@VP-DS VP-DS changed the base branch from feature/translation-module to feature/translation-module2 January 11, 2024 09:54
@VP-DS VP-DS changed the base branch from feature/translation-module2 to feature/translation-module January 11, 2024 09:54
@thomasdax98 thomasdax98 changed the base branch from feature/translation-module to main January 12, 2024 07:50
@thomasdax98 thomasdax98 changed the base branch from main to feature/translation-module January 12, 2024 07:50
@VP-DS VP-DS changed the base branch from feature/translation-module to feature/translation-module2 January 12, 2024 09:22
@VP-DS VP-DS changed the base branch from feature/translation-module2 to feature/translation-module January 12, 2024 09:22
@VP-DS VP-DS marked this pull request as ready for review January 12, 2024 09:39
Copy link
Member

@thomasdax98 thomasdax98 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The whole XML logic looks very complicated. I didn't really review that as I think it's not possible for me to understand it completely without investing hours and testing it first hand.

I trust that you tested it thoroughly for different cases @VP-DS

@VP-DS
Copy link
Contributor Author

VP-DS commented Jan 17, 2024

The whole XML logic looks very complicated. I didn't really review that as I think it's not possible for me to understand it completely without investing hours and testing it first hand.

I trust that you tested it thoroughly for different cases @VP-DS

@thomasdax98 I tested it by trying out all available stylings (see video of #1587)

@VP-DS VP-DS closed this Jan 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants