feat: Initial Scaffold for QTI editor components and add a development page by Abhishek-Punhani · Pull Request #5963 · learningequality/studio

Abhishek-Punhani · 2026-06-09T19:00:01Z

Summary

This PR introduces the initial frontend scaffolding for a brand-new QTI 3.0 authoring editor.

References

Closes #5961

Reviewer guidance

Navigate to any channel in the Studio channel editor.
Replace the end of the URL with /#/qti-demo.
You will see the new standalone QTIEditor loaded with three hardcoded questions.

AI usage

Used Antigravity for final review and nitpicks.

learning-equality-bot · 2026-06-09T19:00:48Z

👋 Hi @Abhishek-Punhani, thanks for contributing!

For the review process to begin, please verify that the following is satisfied:

Contribution is aligned with our contributing guidelines
Pull request description has correctly filled AI usage section & follows our AI guidance:
AI guidance

State explicitly whether you didn't use or used AI & how.

If you used it, ensure that the PR is aligned with Using AI as well as our DEEP framework. DEEP asks you:
- Disclose — Be open about when you've used AI for support.
- Engage critically — Question what is generated. Review code for correctness and unnecessary complexity.
- Edit — Review and refine AI output. Remove unnecessary code and verify it still works after your edits.
- Process sharing — Explain how you used the AI so others can learn.
Examples of good disclosures:

"I used Claude Code to implement the component, prompting it to follow the pattern in ComponentX. I reviewed the generated code, removed unnecessary error handling, and verified the tests pass."

"I brainstormed the approach with Gemini, then had it write failing tests for the feature. After reviewing the tests, I used Claude Code to generate the implementation. I refactored the output to reduce verbosity and ran the full test suite."

Also check that issue requirements are satisfied & you ran pre-commit locally.

Pull requests that don't follow the guidelines will be closed.

Reviewer assignment can take up to 2 weeks.

learning-equality-bot · 2026-06-09T19:50:40Z

📢✨ Before we assign a reviewer, we'll turn on @rtibblesbot to pre-review. Its comments are generated by an LLM, and should be evaluated accordingly.

rtibblesbot

Clean, well-scoped scaffold — the Composition API architecture, immutable update pattern, and i18n discipline all land well.

CI passing. Screenshot shows all three hardcoded cards rendering with working up/down/delete/add controls. All acceptance criteria met.

suggestion (3): canEdit prop is inert with no keyboard path to open cards; QTIItemEditor missing emits declaration (inconsistent with QTIEditor); transition-group animation CSS missing
nitpick (1): setTimeout vs nextTick in addItem

@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly

How was this generated?

Reviewed the pull request diff checking for:

Correctness: bugs, edge cases, undocumented behavior, resource leaks, hardcoded values
Design: unnecessary complexity, naming, readability, comment accuracy, redundant state
Architecture: duplicated concerns, minimal interfaces, composition over inheritance
Testing: behavior-based assertions, mocks only at hard boundaries, accurate coverage
Completeness: missing dependencies, unupdated usages, i18n, accessibility, security
Principles: DRY (same reason to change), SRP, Rule of Three (no premature abstraction)
Checked CI status and linked issue acceptance criteria
For UI changes: inspected screenshots for layout, visual completeness, and consistency

rtibblesbot · 2026-06-09T19:57:03Z

+          :displayMenu="true"
+          :canMoveUp="canMoveUp"
+          :canMoveDown="canMoveDown"
+          :canEdit="!isOpen"


suggestion: :canEdit="!isOpen" is silently inert. AssessmentItemToolbar only uses the canEdit prop to gate the EDIT_ITEM action, but EDIT_ITEM is not included in either iconActionsConfig or menuActionsConfig here. The prop has no runtime effect.

The larger consequence: there is no keyboard-accessible way to open a closed card. The only open path is @click.native="onCardClick" on KPageContainer (a non-focusable <div>), which doesn't respond to Enter/Space. Consider adding [AssessmentItemToolbarActions.EDIT_ITEM, { collapse: true }] to iconActionsConfig when the card is closed, and removing the now-dead :canEdit binding.

rtibblesbot · 2026-06-09T19:57:03Z

+  };
+
+  export default defineComponent({
+    name: 'QTIItemEditor',


suggestion: This component emits open, close, and action but has no emits option declared. QTIEditor (the sibling) correctly has emits: ['update']. For consistency and to give tooling the event contract, add:

emits: ['open', 'close', 'action'],

that'd be a nice addition.

rtibblesbot · 2026-06-09T19:57:03Z

+        </div>
+      </KPageContainer>
+
+      <transition-group


suggestion: transition-group name="list-complete" requires corresponding CSS classes (.list-complete-enter, .list-complete-leave-to, .list-complete-move, etc.) to produce any animation. None are defined in the scoped styles, so the transition silently does nothing. Either add the CSS or remove the name attribute until the animation is ready to implement.

rtibblesbot · 2026-06-09T19:57:03Z

+        list.splice(pos, 0, newItem);
+        emit('update', list);
+        // Open the newly created card on the next tick
+        setTimeout(() => {


nitpick: setTimeout(() => { activeId.value = newItem.id; }, 0) works, but nextTick (imported from 'vue') is the idiomatic Vue way to defer until after the DOM update cycle and is more explicit about intent.

rtibblesbot · 2026-06-09T19:57:03Z

+        activeId.value = null;
+      }
+
+      const cloneList = () => [...props.assessments];


praise: The cloneList() + emit pattern throughout addItem, deleteItem, moveItemUp, moveItemDown is exactly right for the Vuex-independent prop-down/event-up contract — the parent always gets a fresh array and the component never mutates the prop directly.

rtibblesbot · 2026-06-09T19:57:03Z

+const NAMESPACE = 'QTIEditorStrings';
+
+const MESSAGES = {
+  noQuestionsPlaceholder: {


praise: Every string has both message and context, which is what translators need to produce accurate, in-context translations. Good discipline at the scaffolding stage.

AlexVelezLl

Thanks @Abhishek-Punhani! I've found some areas for improvement and a couple of things that weren't specified in the original issue 😅.

AlexVelezLl · 2026-06-09T21:41:59Z

  TRASH: 'TRASH',
  ADD_PREVIOUS_STEPS: 'ADD_PREVIOUS_STEPS',
  ADD_NEXT_STEPS: 'ADD_NEXT_STEPS',
+  QTI_DEMO: 'QTI_DEMO',


Oh, let's just use a hardcoded string 😅, so that we don't forget to remove this constant later.

AlexVelezLl · 2026-06-09T21:42:51Z

+  >
+    <div
+      class="question-card-header"
+      :style="{ borderBottom: isOpen ? `1px solid ${$themePalette.grey.v_200}` : 'none' }"


Could we use $themeTokens.fineLine instead?

AlexVelezLl · 2026-06-09T21:53:05Z

+        <AssessmentItemToolbar
+          :iconActionsConfig="iconActionsConfig"
+          :menuActionsConfig="menuActionsConfig"
+          :displayMenu="true"
+          :canMoveUp="canMoveUp"
+          :canMoveDown="canMoveDown"
+          :canEdit="!isOpen"
+          :collapse="windowIsSmall"
+          :itemLabel="toolbarItemLabel"
+          analyticsLabel="QTI Question"
+          data-test="toolbar"
+          @click="action => $emit('action', action)"
+        />


Ah, apologies, I didn't spec this. Could we create a new CollapsibleToolbar component in the components folder? Let's make this component more generic and reusable, and instead of having so many hardcoded "canMoveUp", "canMoveDown", etc. props, let's make this component receive a single array with the structure:

[{ "icon": "", (string or null) "label": "", (required) "handler": () => {}, "collapsed": true/false }]

So... collapsed will determine if this should go into the dropdown menu; the dropdown menu should only be visible if there is at least one item collapsed. For items that are always collapsed, "collapsed": true; if it depends, we can write the condition here, e.g., "collapsed": windowIsSmall.value. And we can filter out any actions that are not needed (e.g., the first item should not have move up).

AlexVelezLl · 2026-06-09T21:55:32Z

+
+  import { computed, defineComponent } from 'vue';
+  import useKResponsiveWindow from 'kolibri-design-system/lib/composables/useKResponsiveWindow';
+  import { useQTIStr } from '../../qtiEditorStrings';


Could we use the same structure as the communityChannelsStrings file? Where the whole translator is exposed as a named export, and we use destructuring to get the values in the setup. e.g. here.

AlexVelezLl · 2026-06-09T21:56:15Z

+  import AssessmentItemToolbar from 'frontend/channelEdit/components/AssessmentItemToolbar';
+  import { AssessmentItemToolbarActions } from 'frontend/channelEdit/constants';


In general, we should always try to avoid importing from other modules outside the QTIEditor as much as possible.

AlexVelezLl · 2026-06-09T22:14:13Z

+      const { windowIsSmall } = useKResponsiveWindow();
+
+      const containerStyle = computed(() =>
+        windowIsSmall.value ? {} : { maxWidth: '85%', margin: '0 auto' },


Some visual specs have changed (apologies 😅), now maxWidth should be a plain 1200px and the padding/margin should be 16px on small screens and 32px on other screens.

AlexVelezLl · 2026-06-09T22:16:40Z

+        activeId.value = null;
+      }
+
+      const cloneList = () => [...props.assessments];


cloneList isn't fully descriptive, and we are not deep cloning the list; just wondering if it'd be clearer just to write [...props.assessments] where we need this.

AlexVelezLl · 2026-06-09T22:17:52Z

+        // Open the newly created card on the next tick
+        setTimeout(() => {
+          activeId.value = newItem.id;
+        }, 0);


We don't really need to wait until the next tick, right? we can just set the activeId and whenever the list is updated, the activeId will be set.

AlexVelezLl · 2026-06-09T22:18:14Z

+        if (activeId.value === item.id) closeItem();
+        emit(
+          'update',
+          cloneList().filter(i => i.id !== item.id),


Here we dont really need to clone the list because filter already returns a new list.

AlexVelezLl · 2026-06-09T22:20:55Z

+      function onItemAction(action, item, idx) {
+        switch (action) {
+          case AssessmentItemToolbarActions.EDIT_ITEM:
+            openItem(item.id);
+            break;
+          case AssessmentItemToolbarActions.DELETE_ITEM:
+            deleteItem(item);
+            break;
+          case AssessmentItemToolbarActions.ADD_ITEM_ABOVE:
+            addItem({ atIndex: idx });
+            break;
+          case AssessmentItemToolbarActions.ADD_ITEM_BELOW:
+            addItem({ atIndex: idx + 1 });
+            break;
+          case AssessmentItemToolbarActions.MOVE_ITEM_UP:
+            moveItemUp(idx);
+            break;
+          case AssessmentItemToolbarActions.MOVE_ITEM_DOWN:
+            moveItemDown(idx);
+            break;
+        }
+      }


I think it may be easier for us to just use a toolbarActions slot on the QTIItemEditor and manage all these actions directly on the QTIEditor component, which is the component that actually handles the actions.

Yeah great ! I have implemented that and have added a seperate file for actions config

…t page Signed-off-by: Abhishek-Punhani <punhani.manavabhi@gmail.com>

…handlers Signed-off-by: Abhishek-Punhani <punhani.manavabhi@gmail.com>

Signed-off-by: Abhishek-Punhani <punhani.manavabhi@gmail.com>

Abhishek-Punhani · 2026-06-10T18:51:41Z

@AlexVelezLl, currently, I am rendering the icons; for example, the move-up icon is not rendered. Should we add it in the disabled state, or is it fine as is?

AlexVelezLl · 2026-06-10T19:25:34Z

That'd be fine @Abhishek-Punhani, thanks!

learning-equality-bot · 2026-06-10T22:14:41Z

📢✨ Before we assign a reviewer, we'll turn on @rtibblesbot to pre-review. Its comments are generated by an LLM, and should be evaluated accordingly.

AlexVelezLl

Looking much better! Mostly comments about some team unwritten conventions 😅.

AlexVelezLl · 2026-06-11T03:33:09Z

+const KIconButtonStub = {
+  name: 'KIconButton',
+  props: ['icon', 'tooltip', 'ariaLabel', 'disabled', 'color'],
+  template: `
+    <button
+      :data-icon="icon"
+      :aria-label="ariaLabel"
+      :disabled="disabled"
+      @click="$emit('click')"
+    >
+      <slot name="menu" />
+    </button>
+  `,
+};
+
+const KDropdownMenuStub = {
+  name: 'KDropdownMenu',
+  props: ['options'],
+  template: `<div class="k-dropdown-menu"><slot /></div>`,
+  methods: {
+    // expose a helper so tests can directly trigger a select
+    selectOption(value) {
+      this.$emit('select', { value });
+    },
+  },
+};


I think we should be able to test this without stubs.

AlexVelezLl · 2026-06-11T03:35:24Z

@@ -0,0 +1,147 @@
+import { mount } from '@vue/test-utils';


Oh, another little thing 😅. Now we prefer vue-testing-library for new frontend tests.

😅, Yeah I will update the tests to use vue-testing-library

AlexVelezLl · 2026-06-11T03:36:23Z

+
+  import { computed, defineComponent } from 'vue';
+
+  export default defineComponent({


Lets use the export single object sintax instead (without defineComponent) export default {

AlexVelezLl · 2026-06-11T04:02:19Z

+      const dropdownOptions = computed(() => {
+        return collapsedMenuActions.value.map(action => ({
+          label: action.label,
+          value: action.id,
+          disabled: action.disabled,
+        }));
+      });
+
+      function handleSelect(option) {
+        const action = collapsedMenuActions.value.find(a => a.id === option.value);
+        if (action && action.handler) {
+          action.handler();
+        }
+      }


We don't really need an ID for the action (at least not yet). If it is because of handleSelect, there is a better way: We can just pass the handler to the dropdownOptions array.

Then, on the handleSelect we can just do option.handler() :). So there is no need to have an id and do other iteration in the ' handleSelect '.

AlexVelezLl · 2026-06-11T04:05:13Z

+      optionsLabel: {
+        type: String,
+        default: 'Options',
+      },


Let's default it to null, and let's use an optionsLabel$ directly on the template instead. I dont think we have an optionsLabel defined yet, so we can create a new one in the commonStrings module.

AlexVelezLl · 2026-06-11T04:24:45Z

+      /** 1-based position in the list */
+      index: {
+        type: Number,
+        required: true,
+      },


If we are calling it index, then we should probably just get the 0-based position and add 1 if we are displaying it to the user 😅. If we ever need the actual index position, it'll be awkward to subtract 1 for that.

AlexVelezLl · 2026-06-11T04:27:36Z

+      <transition-group
+        name="list-complete"
+        tag="div"
+        class="question-list"
+      >


Transition animations look a bit weird; I think we can just leave them out.

Grabacion.de.pantalla.2026-06-10.a.la.s.11.26.49.p.m.mov

AlexVelezLl · 2026-06-11T04:43:33Z

+<script>
+
+  import { ref, computed, defineComponent } from 'vue';
+  import { v4 as uuidv4 } from 'uuid';


Could we copy this method here instead? This is the way IDs are generated in Studio right now. An alternative would be to accept an idGenerator as a prop, so that we can keep the logic for id generators external to the editor.

I think either way, we will probably need to expose the new and remove events anyway, because we will need this specific info to fit the change architecture in Studio, but I think we can live with this for now and figure out when we do the integration with Studio. For now, ensuring that the IDs generated here are the same as the IDs generated in Studio is enough :).

AlexVelezLl · 2026-06-11T04:46:56Z

+        noQuestionsPlaceholder: noQuestionsPlaceholder$(),
+        newQuestionBtnLabel: newQuestionBtnLabel$(),
+        showAnswersLabel: showAnswers$(),
+        containerStyle,
+        items,
+        activeId,
+        displayAnswersPreview,
+        closeItem,
+        addItem,
+        getToolbarActions,
+        optionsLabel: options$(),


Idem, let's just return the translator function instead.

Also, another undocumented "convention": we usually tend to leave the translator methods at the bottom of the return object, just to make it more readable 😅. Not super important for now (if it was, we should have a linter rule), but to let you know.

AlexVelezLl · 2026-06-11T04:50:37Z

+    if (idx > 0) {
+      result.push({
+        id: 'move-up',
+        icon: 'chevronUp',
+        label: toolbarLabelMoveUp$(),
+        handler: () => moveItemUp(idx),
+        collapsed: windowIsSmall.value,
+      });
+    }
+
+    if (idx < items.value.length - 1) {
+      result.push({
+        id: 'move-down',
+        icon: 'chevronDown',
+        label: toolbarLabelMoveDown$(),
+        handler: () => moveItemDown(idx),
+        collapsed: windowIsSmall.value,
+      });
+    }


Soo... After playing with it a bit, i think it looks weird if the edit button is at different levels. So we probably want to just disable them 😅. So that icons layout are uniform.

rtibblesbot

All four prior findings resolved. No new findings from me — @AlexVelezLl's latest review already covers the open items comprehensively.

CI passing. Screenshot in PR description appears to predate the CollapsibleToolbar introduction, but the rendering logic is straightforward to verify from the code.

Prior findings

Resolved:

:canEdit="!isOpen" silently inert / no keyboard path (suggestion) — replaced wholesale with useQTIEditorActions + slot-based CollapsibleToolbar; edit button is now a keyboard-accessible KIconButton
Missing emits declaration in QTIItemEditor (suggestion) — now has emits: ['close']
transition-group name="list-complete" had no corresponding CSS (suggestion) — animation classes added in QTIEditor/index.vue
setTimeout in addItem (nitpick) — removed; direct assignment now

4/4 prior findings resolved. 0 re-raised.

@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly

How was this generated?

Compared the current PR state against findings from a prior review:

Retrieved prior bot reviews deterministically via the GitHub API
Classified each prior finding as RESOLVED, UNADDRESSED, ACKNOWLEDGED, or CONTESTED
Only raised NEW findings for newly introduced code
Reviewed the pull request diff checking for correctness, design, architecture, testing, completeness, and adherence to DRY/SRP principles
Checked CI status and linked issue acceptance criteria

rtibblesbot · 2026-06-11T05:08:57Z

+            a =>
+              typeof a.id === 'string' &&
+              typeof a.label === 'string' &&
+              typeof a.handler === 'function',


praise: The prop validator enforces the action interface contract at the component boundary — any caller that omits id, label, or handler gets an immediate Vue warning rather than a silent runtime failure deeper in handleSelect. Good defensive practice for a component that will be reused.

rtibblesbot · 2026-06-11T05:08:57Z

+      </h3>
+
+      <div class="question-card-actions toolbar">
+        <slot name="toolbarActions"></slot>


praise: The toolbarActions slot cleanly decouples the card shell from the action model — QTIItemEditor has no knowledge of what actions exist or how they're triggered, and QTIEditor (the right place) owns all list-mutation logic. This is exactly the right boundary for a component that will accumulate editing features over time.

AlexVelezLl requested a review from rtibblesbot June 9, 2026 19:49

rtibblesbot reviewed Jun 9, 2026

View reviewed changes

AlexVelezLl self-requested a review June 9, 2026 20:04

AlexVelezLl self-assigned this Jun 9, 2026

AlexVelezLl requested changes Jun 9, 2026

View reviewed changes

Abhishek-Punhani added 3 commits June 11, 2026 00:20

feat: Initial Scaffold for QTI editor components and add a developmen…

d189d84

…t page Signed-off-by: Abhishek-Punhani <punhani.manavabhi@gmail.com>

feat: implement collapsible QTI editor toolbar and associated action …

9c4f5f9

…handlers Signed-off-by: Abhishek-Punhani <punhani.manavabhi@gmail.com>

test: add unit tests for CollapsibleToolbar and QTIItemEditor components

e85e2e7

Signed-off-by: Abhishek-Punhani <punhani.manavabhi@gmail.com>

Abhishek-Punhani force-pushed the Issue5961 branch from 31a8d05 to e85e2e7 Compare June 10, 2026 18:50

Abhishek-Punhani requested a review from AlexVelezLl June 10, 2026 18:52

AlexVelezLl requested a review from rtibblesbot June 10, 2026 22:14

AlexVelezLl reviewed Jun 11, 2026

View reviewed changes

rtibblesbot reviewed Jun 11, 2026

View reviewed changes

		import AssessmentItemToolbar from 'frontend/channelEdit/components/AssessmentItemToolbar';
		import { AssessmentItemToolbarActions } from 'frontend/channelEdit/constants';


		import { computed, defineComponent } from 'vue';

		export default defineComponent({

Uh oh!

Conversation

Abhishek-Punhani commented Jun 9, 2026

Summary

References

Reviewer guidance

AI usage

Uh oh!

learning-equality-bot Bot commented Jun 9, 2026

Uh oh!

learning-equality-bot Bot commented Jun 9, 2026

Uh oh!

rtibblesbot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlexVelezLl left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Abhishek-Punhani commented Jun 10, 2026

Uh oh!

AlexVelezLl commented Jun 10, 2026

Uh oh!

learning-equality-bot Bot commented Jun 10, 2026

Uh oh!

AlexVelezLl left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

AlexVelezLl Jun 11, 2026 •

edited

Loading