Update Assessment Editor layout and card structure#5924
Update Assessment Editor layout and card structure#5924Abhishek-Punhani wants to merge 2 commits into
Conversation
Signed-off-by: Abhishek-Punhani <punhani.manavabhi@gmail.com>
Signed-off-by: Abhishek-Punhani <punhani.manavabhi@gmail.com>
|
👋 Hi @Abhishek-Punhani, 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. |
|
📢✨ Before we assign a reviewer, we'll turn on |
rtibblesbot
left a comment
There was a problem hiding this comment.
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.vueandAssessmentItemPreview.vue— see inline comment. - suggestion: Fragile negative margin on hints divider — see inline comment.
- praise: Defensive
kindLabelnull 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 { |
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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]) { |
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
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.
Summary
Updated the Assessment Editor Layout according to the new design
This PR:
VCardlayout withKPageContainerfor individual question cards and the "Show answers" checkbox.AssessmentItemPreviewto mirror the new card structure, showing "Question X of Y — [Question type]" in the header.KSelectand adds a "Type" label above it.TipTapEditorand adds a visual divider line before the Hints section.References
Fixes #5912
Reviewer guidance
To test these changes:
AI usage
Codex assisted me in implementing the changes and final nitpicking. I have carefully reviewed all the changes.