Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export const RouteNames = {
TRASH: 'TRASH',
ADD_PREVIOUS_STEPS: 'ADD_PREVIOUS_STEPS',
ADD_NEXT_STEPS: 'ADD_NEXT_STEPS',
QTI_DEMO: 'QTI_DEMO',

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

};

export const viewModes = {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
<template>

<div>
<div style="padding: 16px 24px 0">
<div
style="
padding: 16px;
color: #2196f3;
background-color: transparent;
border: 1px solid #2196f3;
border-radius: 4px;
"
>
<strong>QTI Editor — Dev Demo</strong>
&nbsp;Hardcoded items. Changes are local only and not persisted.
</div>
</div>

<QTIEditor
:assessments="assessments"
@update="onUpdate"
/>
</div>

</template>


<script>

import { ref, defineComponent } from 'vue';
import QTIEditor from 'shared/views/QTIEditor/index';
import { QtiInteraction } from 'shared/views/QTIEditor/constants';

/**
* Hardcoded items covering three interaction types so the closed-card
* type label can be visually verified.
*/
const INITIAL_ASSESSMENTS = [
{
id: 'demo-item-1',
type: QtiInteraction.CHOICE,
title: 'Which planet is closest to the Sun?',
},
{
id: 'demo-item-2',
type: QtiInteraction.EXTENDED_TEXT,
title: 'Describe the water cycle in your own words.',
},
{
id: 'demo-item-3',
type: QtiInteraction.ORDER,
title: 'Arrange these events in chronological order.',
},
];

export default defineComponent({
name: 'QTIDemoPage',

components: { QTIEditor },

setup() {
const assessments = ref(INITIAL_ASSESSMENTS);

function onUpdate(newList) {
assessments.value = newList;
}

return { assessments, onUpdate };
},
});

</script>
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import TrashModal from './views/trash/TrashModal';
import SearchOrBrowseWindow from './views/ImportFromChannels/SearchOrBrowseWindow';
import ReviewSelectionsPage from './views/ImportFromChannels/ReviewSelectionsPage';
import EditModal from './components/edit/EditModal';
import QTIDemoPage from './pages/QTIDemoPage';
import ChannelDetailsModal from 'shared/views/channel/ChannelDetailsModal';
import ChannelModal from 'shared/views/channel/ChannelModal';
import { RouteNames as ChannelRouteNames } from 'frontend/channelList/constants';
Expand Down Expand Up @@ -244,6 +245,11 @@ const router = new VueRouter({
});
},
},
{
name: RouteNames.QTI_DEMO,
path: '/qti-demo',
component: QTIDemoPage,
},
{
name: RouteNames.TREE_VIEW,
path: '/:nodeId/:detailNodeId?',
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,231 @@
<template>

<KPageContainer
noPadding
:topMargin="0"
class="item question-card"
:class="{ closed: !isOpen }"
data-test="item"
@click.native="onCardClick"
>
<div
class="question-card-header"
:style="{ borderBottom: isOpen ? `1px solid ${$themePalette.grey.v_200}` : 'none' }"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we use $themeTokens.fineLine instead?

>
<h3
class="question-card-title"
:style="{ color: $themePalette.grey.v_800 }"
>
<template v-if="isOpen">
{{ questionNumberLabel }}
</template>
<template v-else>
{{ questionNumberAndTypeLabel }}
</template>
</h3>

<div class="question-card-actions toolbar">
<AssessmentItemToolbar
:iconActionsConfig="iconActionsConfig"
:menuActionsConfig="menuActionsConfig"
:displayMenu="true"
:canMoveUp="canMoveUp"
:canMoveDown="canMoveDown"
:canEdit="!isOpen"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

:collapse="windowIsSmall"
:itemLabel="toolbarItemLabel"
analyticsLabel="QTI Question"
data-test="toolbar"
@click="action => $emit('action', action)"
/>
Comment on lines +28 to +40

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

</div>
</div>

<div
v-if="isOpen || displayAnswersPreview"
class="question-card-body"
>
<p :style="{ color: $themePalette.grey.v_500, margin: 0, fontStyle: 'italic' }">
{{ questionContentPlaceholder }}
</p>
</div>

<div
v-if="isOpen"
class="question-card-footer"
>
<KButton
:text="closeBtnLabel"
class="close-item-btn"
data-test="closeBtn"
@click="$emit('close')"
/>
</div>
</KPageContainer>

</template>


<script>

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

import { QtiInteraction } from '../../constants';
import AssessmentItemToolbar from 'frontend/channelEdit/components/AssessmentItemToolbar';
import { AssessmentItemToolbarActions } from 'frontend/channelEdit/constants';
Comment on lines +75 to +76

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


// QTI XML element name → i18n string key, used to build closed-card labels.
const INTERACTION_TYPE_STRING_KEY = {
[QtiInteraction.CHOICE]: 'interactionTypeChoice',
[QtiInteraction.ORDER]: 'interactionTypeOrder',
[QtiInteraction.MATCH]: 'interactionTypeMatch',
[QtiInteraction.TEXT_ENTRY]: 'interactionTypeTextEntry',
[QtiInteraction.EXTENDED_TEXT]: 'interactionTypeExtendedText',
};

export default defineComponent({

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's remove this defineComponent; it is implicit when we export an object from a Vue SFC.

name: 'QTIItemEditor',

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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'],

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that'd be a nice addition.


components: { AssessmentItemToolbar },

setup(props, { emit }) {
const { windowIsSmall } = useKResponsiveWindow();

const questionNumberLabel = computed(() =>
useQTIStr('questionNumberLabel', {
number: props.index,
total: props.total,
}),
);

const questionNumberAndTypeLabel = computed(() => {
const typeKey = INTERACTION_TYPE_STRING_KEY[props.item.type];
const typeLabel = typeKey ? useQTIStr(typeKey) : useQTIStr('interactionTypeUnknown');
return useQTIStr('questionNumberAndTypeLabel', {
number: props.index,
total: props.total,
type: typeLabel,
});
});
const toolbarItemLabel = useQTIStr('toolbarItemLabel');
const closeBtnLabel = useQTIStr('closeBtnLabel');
const questionContentPlaceholder = useQTIStr('questionContentPlaceholder');
Comment on lines +111 to +113

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was there anywhere where we used translators like this? Looks cool, although it's not the new standard we've been using lately.


const canMoveUp = computed(() => props.index > 1);
const canMoveDown = computed(() => props.index < props.total);

const iconActionsConfig = [
[AssessmentItemToolbarActions.MOVE_ITEM_UP, { collapse: true }],
[AssessmentItemToolbarActions.MOVE_ITEM_DOWN, { collapse: true }],
];
const menuActionsConfig = [
AssessmentItemToolbarActions.ADD_ITEM_ABOVE,
AssessmentItemToolbarActions.ADD_ITEM_BELOW,
AssessmentItemToolbarActions.DELETE_ITEM,
];

function onCardClick(event) {
if (props.isOpen) return;
if (
event.target.closest('.toolbar') !== null ||
event.target.closest('.close-item-btn') !== null
) {
return;
}
emit('open');
}
Comment on lines +128 to +137

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, so, this is something we can do differently now. The problem with this is that Keyboard navigation is not possible. So, instead, let's remove the click handler on the entire card, and let's just add an edit button on the toolbar when it is closed. So, let's keep the only way to open the edit mode just being triggered by that edit button.


return {
windowIsSmall,
questionNumberLabel,
questionNumberAndTypeLabel,
toolbarItemLabel,
closeBtnLabel,
questionContentPlaceholder,
canMoveUp,
canMoveDown,
iconActionsConfig,
menuActionsConfig,
onCardClick,
};
},

props: {
/** Assessment item: { id, type (QtiInteraction value), title } */
item: {
type: Object,
required: true,
},
/** 1-based position in the list */
index: {
type: Number,
required: true,
},
/** Total items in the list */
total: {
type: Number,
required: true,
},
/** Whether this card is currently expanded */
isOpen: {
type: Boolean,
default: false,
},
Comment on lines +170 to +174

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of isOpen, let's call it mode with possible values as view/edit, similar to the mode prop on the TipTapEditor. The same rules apply (e.g., just one item in write mode at a time); the only difference now is that this is called mode. (we will be using this wording with many components in the tree)

/** Whether to show answers previews for closed items */
displayAnswersPreview: {
type: Boolean,
default: false,
},
},
});

</script>


<style lang="scss" scoped>

.question-card {
--question-card-horizontal-padding: 20px;

position: relative;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need it to be position: relative?

min-height: 75px;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can remove this min-height.

padding: 0;
margin-bottom: 16px;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's render a column flex container and add a gap in the parent instead! So that this component is more layout-independent :)


&.closed {
cursor: pointer;
}
Comment on lines +196 to +198

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can remove this now.

}

.question-card-header {
display: flex;
align-items: center;
justify-content: space-between;
padding: 12px var(--question-card-horizontal-padding);
}

.question-card-title {
margin: 0;
font-size: 14px;
font-weight: 600;
}

.question-card-actions {
display: flex;
gap: 8px;
align-items: center;
}

.question-card-body {
min-width: 0;
padding: 10px var(--question-card-horizontal-padding);
}

.question-card-footer {
display: flex;
justify-content: flex-end;
padding: 0 var(--question-card-horizontal-padding) 20px;
}

</style>
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
export const Cardinality = Object.freeze({
SINGLE: 'single',
MULTIPLE: 'multiple',
ORDERED: 'ordered',
RECORD: 'record',
});

export const BaseType = Object.freeze({
IDENTIFIER: 'identifier',
BOOLEAN: 'boolean',
INTEGER: 'integer',
FLOAT: 'float',
STRING: 'string',
POINT: 'point',
PAIR: 'pair',
DIRECTED_PAIR: 'directedPair',
DURATION: 'duration',
FILE: 'file',
URI: 'uri',
});

export const QtiInteraction = Object.freeze({
CHOICE: 'choiceInteraction',
ORDER: 'orderInteraction',
MATCH: 'matchInteraction',
TEXT_ENTRY: 'textEntryInteraction',
EXTENDED_TEXT: 'extendedTextInteraction',
});
Loading
Loading