Feature/web avatar loading #65
Conversation
| if (hasCurrentCharacter && props.currentCharacter) { | ||
| const assetsExist = checkAgainstCollection(props.collectionData, props.currentCharacter); | ||
| if (assetsExist) { | ||
| console.log("here! we have a current character!"); |
There was a problem hiding this comment.
leftover dev comment
|
|
||
| export type LoadingErrors = string[]; | ||
|
|
||
| export const parseMMLDescription = ( |
There was a problem hiding this comment.
This should be a library function in 3d-web-avatar
| } from "./test-data"; | ||
| import { extractNumberFromErrorMessage } from "./test-utils"; | ||
|
|
||
| describe("WebAvatarClient Test Utils", () => { |
There was a problem hiding this comment.
These tests could include the parsed character data as well as the errors
| }); | ||
| }); | ||
|
|
||
| describe("WebAvatarClient MML Parsing", () => { |
There was a problem hiding this comment.
I'd suggest simplifying these tests to be an object contents equality check rather than pattern matching. It would be easier to see what the expected outcome was and would be less brittle, even if it were more code.
There was a problem hiding this comment.
The tests now include the parsed data compared to the expected outcome through a toStrictEqual object comparison.
| @@ -0,0 +1,108 @@ | |||
| export const validMCharacter = ` | |||
There was a problem hiding this comment.
Would be good to include syntactically-invalid strings too. Even if the DOMParser implementation handles this simply it's good to have defined behaviour in case the implementation changes.
There was a problem hiding this comment.
@MarcusLongmuir Can you give me one example of such strings?
| @@ -0,0 +1,43 @@ | |||
| import { CollectionDataType } from "@mml-io/3d-web-avatar-editor-ui"; | |||
There was a problem hiding this comment.
This function/file (findAssetsInCollection) should probably be in the @mml-io/3d-web-avatar-editor-ui package.
It shouldn't be in this package (@mml-io/3d-web-avatar) because this package doesn't have a dependency on the @mml-io/3d-web-avatar-editor-ui package.
…ckage to comply with code review
This PR aims to provide avatar loading through MML
m-characteravatar descriptions.What kind of change does your PR introduce? (check at least one)
Does your PR fulfill the following requirements?