[Remove Vuetify from Studio] Content library filter bar#5758
[Remove Vuetify from Studio] Content library filter bar#5758Swoyamjeetcodes wants to merge 3 commits intolearningequality:unstablefrom
Conversation
|
👋 Thanks for contributing! We will assign a reviewer within the next two weeks. In the meantime, please ensure that:
We'll be in touch! 😊 |
|
📢✨ Before we assign a reviewer, we'll turn on |
|
📢✨ Before we assign a reviewer, we'll invite community pre-review. See the community review guidance for both authors and reviewers. |
rtibblesbot
left a comment
There was a problem hiding this comment.
Clean Vuetify removal — VContainer → scoped CSS, VChip → StudioChip, event wiring updated correctly.
CI passing. No dev server available for visual verification.
- blocking: Missing screenshots (acceptance criteria requires them)
- suggestion: Dead
.v-cardCSS could be removed - praise: Correct Vuetify spacing equivalents, good test update
@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
contentcuration/contentcuration/frontend/channelList/views/Channel/CatalogFilterBar.vue
Outdated
Show resolved
Hide resolved
contentcuration/contentcuration/frontend/channelList/views/Channel/CatalogFilterBar.vue
Show resolved
Hide resolved
contentcuration/contentcuration/frontend/channelList/views/Channel/CatalogFilterBar.vue
Show resolved
Hide resolved
...ration/contentcuration/frontend/channelList/views/Channel/__tests__/catalogFilterBar.spec.js
Show resolved
Hide resolved
rtibblesbot
left a comment
There was a problem hiding this comment.
Code changes look correct — Vuetify fully removed, StudioChip wired properly, CSS equivalents accurate, dead .v-card styles cleaned up.
Prior Finding Status
| # | Finding | Severity | Status | Evidence |
|---|---|---|---|---|
| 1 | Missing screenshots | blocking | UNADDRESSED | No screenshots added, no reply |
| 2 | Dead .v-card CSS |
suggestion | RESOLVED | Removed in 51b825d |
| 3 | Correct spacing equivalents | praise | — | — |
| 4 | Good test update | praise | — | — |
Summary: 1/2 actionable prior findings resolved. 1 re-raised below.
CI passing. No dev server available for visual verification.
- blocking: Screenshots still missing (see inline comment)
- praise:
.v-cardcleanup is a nice finishing touch — component is now fully Vuetify-free
@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
contentcuration/contentcuration/frontend/channelList/views/Channel/CatalogFilterBar.vue
Show resolved
Hide resolved
contentcuration/contentcuration/frontend/channelList/views/Channel/CatalogFilterBar.vue
Show resolved
Hide resolved
|
📢✨ Before we assign a reviewer, we'll turn on |
rtibblesbot
left a comment
There was a problem hiding this comment.
All prior findings resolved — screenshots added, .v-card cleanup done. Visual parity confirmed from before/after screenshots. CI passing.
Prior Finding Status
| # | Finding | Severity | Status | Evidence |
|---|---|---|---|---|
| 1 | Missing screenshots | blocking | RESOLVED | Before/after screenshots added to PR description |
| 2 | Dead .v-card CSS |
suggestion | RESOLVED | Removed in 51b825d (confirmed in prior review) |
| 3 | Correct spacing equivalents | praise | — | — |
| 4 | Good test update | praise | — | — |
- praise: Clean, minimal PR — Vuetify fully removed with accurate CSS equivalents and proper
StudioChipintegration
@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
VContainerwith a plain wrapper div and explicit scoped styles for max-width, centering, and padding.VChipwith sharedStudioChipand updated chip-close handling from@inputto@close.StudioChipclose button ([data-test="remove-chip"]) instead of Vuetify’s.v-chip__close.Manual verification performed:
pnpm test -- contentcuration/contentcuration/frontend/channelList/views/Channel/__tests__/catalogFilterBar.spec.js --runInBandPASS(1 suite, 4 tests)References
Reviewer guidance
##Screenshots
AI usage