Fix assessments tab height - #6111
Conversation
🟡 Waiting for changesLast updated: 2026-08-28 16:35 UTC |
rtibblesbot
left a comment
rtibblesbot
left a comment
There was a problem hiding this comment.
PR #6111 — replacing the calc(100vh - 128px) chain with a percentage-height chain is the right direction, and the chain does resolve: VCard → .uploader-container → .v-content → .v-content__wrap → .wrapper → .edit-view-layout → .tab-content, which picks up flex: 1 1 100% from Vuetify's global .container rule (shared/styles/vuetify.scss:7634). CI passing.
- suggestion —
Uploader.vue: shared component getsheight: 100%for one consumer's benefit (inline) - suggestion —
EditModal.vue:64pxduplicatesBottomBar's height with nothing linking them;!importantunexplained; comment ends mid-sentence (inline) - nitpick ×2 — dead
max-height: inherit, possibly redundant wrapper div (inline)
Manual QA did not run on this branch, so nothing here is visually verified. The one case worth exercising before merge is #5938's exact repro — Questions tab, a single collapsed question, tall viewport — since the screenshot in the description shows a long list that fills the column regardless. Worth also confirming the Details tab still scrolls with sticky tab elevation, since .wrapper's bounds now rest entirely on the new chain.
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
Ran a phased review pipeline over the pull request diff:
- Classified the diff to select review passes (core, frontend, backend) and whether manual QA was required
- Core review pass checked correctness, design, architecture, testing, completeness, and DRY/SRP/Rule-of-Three principles
- Specialized frontend/backend review passes applied framework-specific lenses where those files changed
- For UI changes: manual QA and an accessibility audit against a live dev server, when available
- Checked CI status and linked issue acceptance criteria
- Synthesized one review from those passes and chose the verdict from the findings, CI status, and QA evidence
| z-index: 5; | ||
| } | ||
|
|
||
| .edit-view-layout-wrapper { |
There was a problem hiding this comment.
nitpick: Is the extra div doing something? .wrapper already carries a definite height: 100%, so .edit-view-layout's min-height: 100% should resolve against it directly without the intermediate wrapper.
There was a problem hiding this comment.
Yes, it is preventing this PR from getting an unnecessarily large diff; it's on purpose.
3a4e3c9 to
cd59ec8
Compare
rtibblesbot
left a comment
rtibblesbot
left a comment
There was a problem hiding this comment.
PR #6111 — 3 of 5 prior findings resolved; 2 still open, both contested and deferred to you.
EditModal.vue:703—!importantleft unexplained. You declined on the grounds that documenting Vuetify internals will go stale once Vuetify is removed. Reasonable; not pursuing.EditView.vue:448— possibly redundant.edit-view-layout-wrapperdiv. Kept deliberately to hold the diff down. Reasonable; not pursuing.
Re-traced the percentage-height chain against the amended code — no new correctness issues. CI passing. Manual QA did not run on this branch, so nothing here is visually verified; COMMENT rather than approve for that reason alone.
Prior-finding status
RESOLVED — contentcuration/contentcuration/frontend/shared/views/files/Uploader.vue:286 — shared component given height: 100% for one consumer
RESOLVED — contentcuration/contentcuration/frontend/channelEdit/components/edit/EditModal.vue:703 — 64px duplicated BottomBar height with nothing linking them
RESOLVED — contentcuration/contentcuration/frontend/channelEdit/components/edit/EditView.vue:497 — dead max-height: inherit
CONTESTED — contentcuration/contentcuration/frontend/channelEdit/components/edit/EditModal.vue:703 — !important left unexplained in the comment
CONTESTED — contentcuration/contentcuration/frontend/channelEdit/components/edit/EditView.vue:448 — possibly redundant .edit-view-layout-wrapper div
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
Ran an automatic code-only delta review triggered by new commits on a previously reviewed PR:
- Retrieved prior bot reviews via the GitHub API
- Classified each prior finding as RESOLVED, UNADDRESSED, ACKNOWLEDGED, or CONTESTED
- Only raised NEW findings for newly introduced code
- Core review pass only — specialized frontend/backend lenses and manual QA run when a review is explicitly requested
- Synthesized one review from the passes and chose the verdict from the findings, CI status, and QA evidence
|
|
||
| // Height of the fixed BottomBar. Content that sits behind the bar reserves the | ||
| // same amount of room for it. | ||
| $bottom-bar-height: 64px; |
There was a problem hiding this comment.
praise: Defined once and consumed by BottomBar.vue itself, not just by the component that reserves space for it — that's what keeps the two in lockstep.
marcellamaki
left a comment
marcellamaki
left a comment
There was a problem hiding this comment.
code changes, while opinionated 😅 , look okay and the comments are helpful. Brief manual QA checks out, and let's be sure to add this regression testing as an extra check for the QA team when they start working on reviewing the feature
3 participants
Summary
References
Closes #5938.
Reviewer guidance
questionstab.AI usage
I made the changes, as it was a very opinionated frontend change, with Claude explaining what some Vuetify components do.