Skip to content

Update Assessment Editor layout and card structure#5924

Open
Abhishek-Punhani wants to merge 2 commits into
learningequality:unstablefrom
Abhishek-Punhani:Issue5912
Open

Update Assessment Editor layout and card structure#5924
Abhishek-Punhani wants to merge 2 commits into
learningequality:unstablefrom
Abhishek-Punhani:Issue5912

Conversation

@Abhishek-Punhani
Copy link
Copy Markdown
Member

Summary

Updated the Assessment Editor Layout according to the new design

This PR:

  • Replaces the legacy VCard layout with KPageContainer for individual question cards and the "Show answers" checkbox.
  • Introduces a scannable header for active questions displaying "Question X of Y" alongside move controls.
  • Updates the AssessmentItemPreview to mirror the new card structure, showing "Question X of Y — [Question type]" in the header.
  • Migrates the response type selector to KSelect and adds a "Type" label above it.
  • Reduces the default minimum height of the TipTapEditor and adds a visual divider line before the Hints section.
  • Ensures a minimum of 16px vertical separation between all question cards.

References

Fixes #5912

Reviewer guidance

To test these changes:

  1. Open a channel in the Studio channel editor.
  2. Select or create an Exercise node.
  3. In the edit panel, navigate to the Questions tab.
  4. Add a few questions and verify both the preview and editor states.

AI usage

Codex assisted me in implementing the changes and final nitpicking. I have carefully reviewed all the changes.

Signed-off-by: Abhishek-Punhani <punhani.manavabhi@gmail.com>
Signed-off-by: Abhishek-Punhani <punhani.manavabhi@gmail.com>
@learning-equality-bot
Copy link
Copy Markdown

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

@AlexVelezLl AlexVelezLl requested a review from rtibblesbot May 19, 2026 18:46
@learning-equality-bot
Copy link
Copy Markdown

📢✨ 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.

Copy link
Copy Markdown
Contributor

@rtibblesbot rtibblesbot left a comment

Choose a reason for hiding this comment

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

All 11 acceptance criteria are met in the code. CI passing.

Visual verification: the PR has no screenshots and the dev server was not available to generate them — see blocking comment.

Findings:

  • blocking: No screenshots provided for a PR with significant visual changes (card layout, KSelect, background color, headers). Please add before/after screenshots showing the new card structure, question headers, and grey background on the Questions tab.
  • suggestion: CSS duplication between AssessmentEditor.vue and AssessmentItemPreview.vue — see inline comment.
  • suggestion: Fragile negative margin on hints divider — see inline comment.
  • praise: Defensive kindLabel null guard prevents translator crash on unknown item types — see inline comment.
  • praise: Re-querying DOM state after async operations in tests is a good improvement — see inline comment.

@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

cursor: pointer;
}

.question-card-header {
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: .question-card-header and .question-card-title are defined identically in both this file and AssessmentEditor.vue (lines 505-518). Since these are scoped styles the duplication is structurally unavoidable today, but it's worth noting for whoever eventually builds a shared QuestionCard component — these are the styles to pull out.

}

.hints-divider {
margin: 16px -28px 0;
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: The -28px negative margin is coupled to the 28px horizontal padding of .question-card-body in AssessmentEditor.vue. If that padding ever changes, the divider will be misaligned without an obvious link between the two values. A comment noting the coupling (e.g., // matches .question-card-body padding in AssessmentEditor.vue) would prevent future confusion.

return sortBy(this.item.hints, 'order');
},
kindLabel() {
if (!this.kind || !AssessmentItemTypeLabels[this.kind]) {
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.

praise: Good defensive check — the old code would have passed undefined to translator.$tr() for any item type not present in AssessmentItemTypeLabels, which would throw. This null guard (returning '' for unknown types) is the right fix and also silently handles the Perseus case without a separate branch.

it('opens an item on item click', async () => {
const items = getItems(wrapper);
await items.at(1).trigger('click');
const updatedItems = getItems(wrapper);
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.

praise: Re-querying via getItems(wrapper) after each async trigger('click') is the correct pattern — it ensures assertions run against DOM state after Vue's reactivity cycle has flushed. The previous tests were holding onto stale wrappers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update Assessment Editor layout and card structure

2 participants