fix: handle undefined values in answer lists #2204
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -437,7 +437,15 @@ export function parseAnswerList(answerList: any) { | |
| return parseAnswerListItem(item); | ||
| } | ||
| ); | ||
| // Remove any items from the list which only have a value for "name", | ||
| // e.g. "image" and "text" are undefined because the list has been generated within a template | ||
| const filteredAnswerListItems = answerListItems.filter((item: IAnswerListItem) => { | ||
| for (let [key, value] of Object.entries(item)) { | ||
| if (key !== "name" && value !== undefined) return true; | ||
| } | ||
| return false; | ||
| }); | ||
| return filteredAnswerListItems; | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -449,8 +457,9 @@ function parseAnswerListItem(item: any) { | |
| if (typeof item === "string") { | ||
| const stringProperties = item.split("|"); | ||
| stringProperties.forEach((s) => { | ||
| let [field, value] = s.split(":").map((v) => v.trim()); | ||
| if (field && value) { | ||
| if (["undefined", "NaN", "null", '""'].includes(value)) value = undefined; | ||
|
There was a problem hiding this comment. nit(blocking) Additionally I'm not sure converting empty string to undefined makes sense. When thinking of answer lists there's sometimes a case where html components may want to use empty string to represent nothing selected in the context of something like a select dropdown. There was a problem hiding this comment. Yeh I don't think this was sensible, I've updated to only convert the string |
||
| itemObj[field] = value; | ||
| } | ||
| }); | ||
|
|
||
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.
nit(blocking)
This logic seems a bit confusing to me, we're running a filter operation but then have an inner for loop which will escape before completion. Might make more sense as 2-step operation, e.g more like
That being considered, I'm not sure if this makes total sense to implement - surely it's better to let broken images through so that an author can be aware of an authoring mistake instead of just hiding the data away? I'd probably lean more towards providing an authoring option to add a filter function to the specific component, e.g.
filter_no_image:trueor ensuring support for runtime filtering, e.g.@data.some_list.filter(item=>item.image!==undefined)Additionaly we'd still end up with broken images in cases where text is supplied without image with the above filter.
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.
Your suggested logic is definitely clearer, I have refactored accordingly.
In terms of allowing through broken images, a broken image link will still be allowed through with the suggested implementation – an invalid path to an image will still display a broken image link. The
imagewill only be filtered out if the value isundefined– which admittedly could be due to an authoring error that the author should be made aware of, but it's not clear to me that displaying a broken image would be necessary in such cases.Either way, returning to the issue referenced in this PR, we want a way to author an answer list in a generic way where the source data may or may not contain image paths, and in the case where an item does not reference an image, to display no image (and not a broken image link). Filtering on the
undefinedvalues as suggested seems like a satisfactory solution, but I may be missing some associated problems, and there may be other approaches.Ultimately, I think the best solution may be a way of defining arbitrary data structures, or at least answer lists, directly from data lists, handled at parse-time, in a way that can handle lists of multiple length and containing partial values (as in WIP RFC for a universal data list parser). But I think we want an acceptable solution sooner in order to help with the current authoring requirements.