-
Notifications
You must be signed in to change notification settings - Fork 0
feat: add rtf functionality #560
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
base: main
Are you sure you want to change the base?
Changes from all commits
07020e2
ed953f6
493d26f
b4fdb62
9e81d3c
4f9e98c
223e8a8
3030f30
d93920c
d4f2717
433e7d7
3ffe0a9
15c4b4f
e967539
5307aac
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,33 +34,48 @@ export const resolveTranslatableString = ( | |
}; | ||
|
||
/** | ||
* Converts a type TranslatableRichText to a type that can be viewed on the page | ||
* Converts a type TranslatableRichText to string or RTF Element | ||
* @param translatableRichText | ||
* @param locale | ||
*/ | ||
export const resolveTranslatableRichText = ( | ||
translatableRichText?: TranslatableRichText, | ||
locale: string = "en" | ||
): string | React.ReactElement => { | ||
if (!translatableRichText) return ""; | ||
try { | ||
if (!translatableRichText) return ""; | ||
|
||
if ( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's this if condition, the one below, and then the logic in isRichText which also checks if the type is an object like on line 55. Can this be simplified to just check if translatableRichText is a map, grab the locale, and then always call toStringOrElement which happens in both of these conditions? |
||
typeof translatableRichText === "string" || | ||
isRichText(translatableRichText) | ||
) { | ||
return toStringOrElement(translatableRichText); | ||
} | ||
|
||
if ( | ||
typeof translatableRichText === "string" || | ||
isRichText(translatableRichText) | ||
) { | ||
return toStringOrElement(translatableRichText); | ||
} | ||
if (typeof translatableRichText === "object") { | ||
const localizedValue = translatableRichText[locale]; | ||
if (localizedValue) { | ||
return toStringOrElement(localizedValue); | ||
} | ||
} | ||
|
||
const localizedValue = translatableRichText[locale]; | ||
if (localizedValue) { | ||
return toStringOrElement(localizedValue); | ||
return ""; | ||
} catch (error) { | ||
console.warn("Error in resolveTranslatableRichText:", error); | ||
return ""; | ||
} | ||
|
||
return ""; | ||
}; | ||
|
||
function isRichText(value: unknown): value is RichText { | ||
return ( | ||
typeof value === "object" && | ||
value !== null && | ||
("html" in value || "json" in value) | ||
); | ||
} | ||
|
||
/** | ||
* Takes a TranslatableString and a locale and returns the value to be displayed in the editor input | ||
* Takes a TranslatableString and a locale and returns the value as a string | ||
* @param translatableString a TranslatableString | ||
* @param locale "en" or other locale value | ||
* @return string to be displayed in the editor input | ||
|
@@ -75,6 +90,7 @@ export function getDisplayValue( | |
if (!locale) { | ||
locale = "en"; | ||
} | ||
|
||
if (typeof translatableString === "string") { | ||
return translatableString; | ||
} | ||
|
@@ -100,14 +116,6 @@ function richTextToString(rtf: RichText): string { | |
return rtf.html || rtf.json || ""; | ||
} | ||
|
||
function isRichText(value: unknown): value is RichText { | ||
return ( | ||
typeof value === "object" && | ||
value !== null && | ||
("html" in value || "json" in value) | ||
); | ||
} | ||
|
||
/** | ||
* Converts a "string | RichText" type to "string | React.ReactElement" which can be viewed on the page | ||
* @param value | ||
|
@@ -118,5 +126,12 @@ function toStringOrElement( | |
if (isRichText(value)) { | ||
return <MaybeRTF data={value} />; | ||
} | ||
return value ?? ""; | ||
|
||
// Ensure we only return strings | ||
if (typeof value === "string") { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, we can't migrate the strings to rtf because we don't know the formatting and that happens in Storm, right? But now the rtf fields from Storm that we're going to render are always the html version so can't we just always pretend it's a string we need to render? Or is this to support the json version from the document because Spruce hasn't switch it over yet? Basically I'm trying to see if we can simplify a lot of the crazy logic in here. Is that possible? If not now, will it be possible at some point, and if so, what is needed to do that? |
||
return value; | ||
} | ||
|
||
// If value is anything else (object, number, etc.), return empty string | ||
return ""; | ||
} |
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.
Once Storm is deployed it will always send the new payload right? If so then you shouldn't need any fallback.