fix: olx parsers with formatted text #336
Conversation
|
preserve-order is not in this PR? and it seems like this PR is just changes we've made in the past week again? |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #336 +/- ##
==========================================
+ Coverage 90.94% 91.05% +0.10%
==========================================
Files 215 215
Lines 3821 3834 +13
Branches 751 745 -6
==========================================
+ Hits 3475 3491 +16
+ Misses 325 323 -2
+ Partials 21 20 -1
☔ View full report in Codecov by Sentry. |
| */ | ||
| getPreservedAnswersAndFeedback(problemType, widgetName, option) { | ||
| const preservedProblem = this.richTextProblem; | ||
| const isChoiceProblem = problemType !== ProblemTypeKeys.NUMERIC ? problemType !== ProblemTypeKeys.TEXTINPUT : false; |
There was a problem hiding this comment.
This is hard to understand. I needed a while to work through all the negations to figure out what value is actually returned. I played around a bit, this seems a more readable version I came up with:
const isChoiceProblem = !([ProblemTypeKeys.NUMERIC, ProblemTypeKeys.TEXTINPUT].includes(problemType))
| } | ||
| preservedProblem.forEach(wrapperTag => { | ||
| const wrapperTagKeys = Object.keys(wrapperTag); | ||
| if (wrapperTagKeys.includes(problemType)) { |
There was a problem hiding this comment.
This function has a very high complexity. There are multiple nested if statements and for loops and it is not possible for me to follow along with the code without spending a large amount of time analyzing it. It seems to me to make this more readable and maintainable, it is helpful to split this up into several well-named functions and variables, and avoid nested ifs / for loops.
There was a problem hiding this comment.
I will mark this as optional. You can decide whether you want to do this here or not. I would of course welcome any improvements of readability, but the parser is already very complex anyway and we can also leave refactoring for another time. With the great amount of work you did in this issue I'm sure you'll be glad to get this merged.
KristinAoki
force-pushed
the
KristinAoki/fix-olx-parser-with-formatted-text
branch
from
b3e0783 to
c6353f5
Compare
JIRA Ticket: TNL-10689
Currently if a user formats their text (italicize, bold, etc.) or adds an image all inside the same parent tag, e.g.
<p>, the order of the text/image will be changed on save and re-open. This causes user to be confused and not trust the visual editor as it is not saving their work as expected. This PR usesfast-xml-parser's attributepreserve-orderto keep the text in the same order while parsing and building. This attribute is currently used for the question parser, and is now being applied to all places where tinymce is used.