feat(QTI): add associate and match interaction viewers - #15151
feat(QTI): add associate and match interaction viewers#15151habibayman wants to merge 17 commits into
Conversation
github-actions
Bot
added
DEV: renderers
habibayman
changed the title
|
👋 Hi @habibayman, thanks for contributing! For the review process to begin, please verify that the following is satisfied:
Also check that issue requirements are satisfied & you ran Pull requests that don't follow the guidelines will be closed. Reviewer assignment can take up to 2 weeks. |
Build Artifacts
Smoke test screenshot |
b82b9cc to
49c46e4
Compare
npm Package VersionsWarning The following packages have changed files but no version bump:
If these changes affect published code, consider bumping the version. |
rtibbles
left a comment
rtibbles
left a comment
There was a problem hiding this comment.
Just a quick first pass review - this seems to be going in a good direction, the main thing that surprised me was the seemingly limited code reuse between Match and Associate - I had assumed because one is essentially the "1 fixed column" form of the other that we would be sharing a lot more code with a one column 'prefilled' and not dynamic. Clearly this is not 3 instances, so there could be a good case to be made for not sharing logic, but they seemed so similar and the specs are so aligned it feels worthy of discussion.
|
|
||
| // Choices are static: parse the slot vnodes once rather than on every render. | ||
| const allContent = (slots.default && slots.default()) || []; | ||
| const nonChoiceContent = allContent.filter(vnode => getComponentTag(vnode) !== CHOICE_TAG); |
There was a problem hiding this comment.
I am squinting my eyes and starting to see things we may be able to reuse here. Rule of three is coming into view!
|
|
||
| const rowCount = computed(() => { | ||
| const max = typedProps.maxAssociations.value; | ||
| // max-associations="0" means unlimited, so offer as many pairs as the |
There was a problem hiding this comment.
I guess at least it was 0 and not -1.
| provide('isSelected', isSelected); | ||
| provide('toggleSelection', toggleSelection); | ||
|
|
||
| const getShuffledOrder = choices => { |
There was a problem hiding this comment.
Reduce, reuse, recycle! Very good.
|
|
||
| <span | ||
| class="qti-simple-associable-choice" | ||
| dir="auto" |
There was a problem hiding this comment.
Is this purely to add dir="auto"? Not objecting, just checking the motivation.
| <template> | ||
|
|
||
| <!-- | ||
| Rendered indirectly: MatchInteraction reads the two sets out of its slot to |
There was a problem hiding this comment.
If there are multiple cases like this, it may be better simply to register "pass through" qti tags to tell DOMPurify to leave them alone.
80a5b77 to
da30398
Compare
habibayman
changed the title
da30398 to
3020bdf
Compare
2 participants
Summary
WIP 🚧
References
Reviewer guidance
AI usage