Conversation
carla-at-wiris
requested review from
jgonzalez-at-wiris,
ncornaglia-at-wiris,
usantos-at-wiris and
xjiang-at-wiris
as code owners
carla-at-wiris
added
type:bug
| const latex = acceptedLatex !== undefined && acceptedLatex !== standardResult?.latex ? acceptedLatex : standardResult?.latex; | ||
| if (latex === undefined && acceptedLatex === undefined) { |
There was a problem hiding this comment.
Estas condiciones son super complejas de entender, sobre todo porque latex ya contiene incluye de alguna forma a acceptedLatex, sin embargo en el if se comprueban a ambos. Me parece retorcido, ¿quizás con un comentario se pueda mejorar?
There was a problem hiding this comment.
Esta comprobación es necesaria, ya que, aunque puedan depender entre ellas, también pueden ser undefined de forma independiente
Aun así, este fichero es el compilado y sobre el que no tenemos ningún control. El que interesa revisar es todo lo que hay fuera de la carpeta dist, que ahora mismo es solo el integration.js
There was a problem hiding this comment.
Le añado un comentario al fichero nuestro explicando esto
jgonzalez-at-wiris
left a comment
jgonzalez-at-wiris
left a comment
There was a problem hiding this comment.
En general la solución no me gusta porque estamos duplicando lógica de algún modo: latex y acceptedLatex. De todas formas veo que eso ya estaba y siendo sincero, el proyecto necesita una revisión general, no vamos a rasgarnos las vestiduras por este detalle ahora mismo.
Echo en falta que el código respire en algunos sitios (saltos de línea tras bloques de código). Por lo demás poco más, he dejado un comentario por ahi!
| // Use explicit undefined check to allow empty LaTeX strings. | ||
| const latex = acceptedLatex !== undefined && acceptedLatex !== standardResult?.latex ? acceptedLatex : standardResult?.latex; | ||
| if (latex === undefined && acceptedLatex === undefined) { |
There was a problem hiding this comment.
Veo que esto es código repetido de lo que hay en index.js. Me imagino el motivo, pero me gustaría saber si para el furuo, esto es algo que se podríia evitar (el builder posiblemente ya sepa cómo consturir la versión umd y la "normal")
There was a problem hiding this comment.
Todo lo que hay en la carpeta dist es el compilado autogenerado por ck5 a partir de nuestro plugin. No tenemos mucho que decir/hacer allí
* fix: Empty LaTeX in CKEditor 5 * docs: Update code docs
3 participants
Description
This PR fixes the following:
$$$$was not being recognized, hence not adding LaTeX content when inserting a formula in between the $$.Type of Change
How should be tested? (Manual or Automated Tests)
Go to the staging demo and validate that the bug is fixed.