Update Assessment Editor layout and card structure#5924
Update Assessment Editor layout and card structure#5924Abhishek-Punhani wants to merge 3 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
…assessment components Signed-off-by: Abhishek-Punhani <punhani.manavabhi@gmail.com>
rtibblesbot
left a comment
There was a problem hiding this comment.
Prior findings
Resolved:
- No screenshots for a PR with significant visual changes (blocking) — screenshots added as PR comment; visual inspection below
- CSS duplication between
AssessmentItemPreview.vueandAssessmentEditor.vue(suggestion) — comment added explaining scoped-style constraint and future extraction path - Fragile
-28pxnegative margin coupling in.hints-divider(suggestion) — comment added documenting the coupling withAssessmentEditor.vue
3/3 prior findings resolved. 0 re-raised.
CI: Build and deploy tests failing on the latest commit — "Build frontend assets" fails at the "Use Node.js" setup step (step 4 of 13) in 16 seconds. All functional checks pass (JS Tests, Python Tests, Linting, Container Build). This is a transient GitHub Actions infrastructure failure unrelated to the code changes. Please re-run the failing job before merging.
Visual inspection: Screenshots provided as PR comment. Screenshot 1 shows KPageContainer card structure, "Question 1 of 1" header with move controls, "Type" label above KSelect, and the grey Questions tab background — all consistent with the target design from issue #5912. Screenshot 2 shows the Answers and Hints sections. No preview-state screenshot provided, but the prior round's code review confirmed AssessmentItemPreview implements the correct structure.
Findings:
- suggestion: Please re-run the failing "Build frontend assets" CI job before merging — it's a transient Node.js setup failure, not a code issue, but the check needs to be green.
- praise: The documentation comments added in this commit are well-written — both the
AssessmentItemPreviewscoped-styles note and theAssessmentItemEditormargin-coupling note are accurate, specific, and include actionable forward-looking guidance ("when a shared QuestionCard component is eventually built, these are the styles to extract into it"). - nitpick: Screenshots were added as a PR comment rather than to the PR description. The PR template asks for screenshots in the description; they're easier to find there during review than in the comment thread.
@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


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.