refactor(ui): tabs spacing/alignment + PageHeader split (CorePageHeaderTabs)#4190
Merged
Merged
Conversation
…low tabs - CoreSurfaceTabBar: add `mx-4` to `<v-tabs>` so the tab strip aligns horizontally with PageHeader's title icon (both now use the same 16px inset inside `v-container fluid`). - user.view.vue + admin.layout.vue: add `pb-0` on the parent `<v-container fluid>` to collapse ~16px of dead space between the tab underline and the routed child's top edge. Child views own their own top padding via their own `<v-container fluid>` + `pa-2` row. Visual impact (Account, Org detail, Admin surfaces): - Tabs now align with the section title icon instead of being 16px to the left - Vertical gap below tabs drops from ~40px to ~24px
Extends the tabs-spacing-alignment fix to organization.detail.component.vue which had the same `<v-container fluid>` parent pattern as user.view.vue and admin.layout.vue. Brings /users/organizations/:id/* into visual parity with /users/profile and /admin/* surfaces.
Splits the chrome primitive into two specialised components so each page gets the spacing it actually needs: - `PageHeader` (existing, simplified): content-sized header for list views and simple pages (Tasks, Scraps, etc.). Drops the 56px min/max-height enforcement and removes the dead `#tabs` slot path (unused since the CoreSurfaceTabBar sibling refactor in #4187/#4188). - `CorePageHeaderTabs` (new): bundles a PageHeader + CoreSurfaceTabBar and enforces the 56px height externally via a scoped `:deep()` rule. Used on tabbed surfaces (Account, Organization detail, Admin) where the title↔ breadcrumb rhythm matters across route transitions. The new primitive accepts `tabs`, `can`, `basePath`, and `hideTabs` for the tab bar, plus passthrough of icon/title/subtitle/avatar/actions/breadcrumb slots and props to the inner PageHeader. Migrates 3 callsites: - user.view.vue (Account) - admin.layout.vue (with hideTabs in breadcrumb mode) - organization.detail.component.vue Visual impact: pages without tabs (e.g. /tasks) no longer pay the 56px chrome tax; tabbed surfaces preserve the route-transition rhythm.
The empty-state v-row was missing `pa-2 mt-0` (so Vuetify's default v-row margin applied) and the card carried `ma-6` (24px all-sides margin), which together added ~36px of dead space above the "No tasks yet" card compared to the populated state. Drop `ma-6` and apply the same `pa-2 mt-0` row classes as the populated branch — empty state now sits flush with where task cards would render.
Drop `align="start" justify="center"` on the v-row and align card padding (pa-8 → pa-6) so the empty-state card sits at the exact same top/left as the form card on /task. Keeps `text-center` for icon + label centering inside the card.
The populated state's v-row had no v-if guard, so when the task list was empty it still rendered (just without children) — taking ~16-24px of vertical space (pa-2 + default v-row margins) on top of the empty-state row below. Empty-state card now sits at the same top position as the populated cards (and /task form card).
…refactor Adds a new migration entry above the previous chrome convergence entry covering the changes in this batch: - PageHeader is content-sized (56px enforcement removed) - #tabs slot on PageHeader dropped (dead since #4187) - mx-4 on v-tabs inside CoreSurfaceTabBar (horizontal alignment with title) - New CorePageHeaderTabs primitive that bundles PageHeader + SurfaceTabBar and re-enforces the 56px rhythm for tabbed surfaces - pb-0 on the tabbed-layout container to collapse the gap below tabs - Empty-state v-row regression pattern (guard the populated row with v-if) Downstream impact: minimal — only projects with custom tabbed layouts need to migrate to CorePageHeaderTabs; default consumers get the new convention via /update-stack automatically.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (13)
WalkthroughThis PR introduces ChangesPageHeader split + CorePageHeaderTabs refactor
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4190 +/- ##
=======================================
Coverage 99.56% 99.56%
=======================================
Files 31 31
Lines 1140 1140
Branches 329 329
=======================================
Hits 1135 1135
Misses 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three layered fixes to the chrome convention that #4187/#4188 established:
CoreSurfaceTabBarnow appliesmx-4on its inner<v-tabs>, so the tab strip aligns with the section title icon (same x-offset ascorePageHeader).user.view.vue,admin.layout.vue,organization.detail.component.vue) getclass="pb-0"to collapse the dead space between the tab underline and the routed child's top edge. ~40px → ~24px.PageHeaderis now content-sized (drop the canonical 56pxmin/max-height) and loses the dead#tabsslot path. A newCorePageHeaderTabsbundlesPageHeader+CoreSurfaceTabBarand re-enforces the 56px title↔breadcrumb rhythm via scoped:deep(). The 3 tabbed callsites migrate to the new primitive.Plus an empty-state regression fix on
tasks.view.vue(the populated v-row had nov-ifguard, so it took ~16-24px of vertical space and pushed the "No tasks yet" card down).Files
core.surfaceTabBar.component.vue—<v-tabs class="mx-4">core.pageHeader.component.vue— content-sized, no#tabsslotcore.pageHeaderTabs.component.vue— new, bundles PageHeader + SurfaceTabBaruser.view.vue— migrate toCorePageHeaderTabsadmin.layout.vue— migrate toCorePageHeaderTabswith:hide-tabsin breadcrumb modeorganization.detail.component.vue— migrate toCorePageHeaderTabstasks.view.vue— empty-state row spacing harmonized with/task+v-ifguard on populated rowCorePageHeaderTabsMIGRATIONS.md— new entry documenting the refactor for downstream projectsTests
core.pageHeaderTabs.component.unit.tests.js: 8/8 pass (composition, breadcrumb forward, hideTabs gating, CASL).#tabsslot + the no-longer-enforced 56px CSS rule.CorePageHeaderTabsinstead ofPageHeader+CoreSurfaceTabBarseparately.Test plan
/users/profile,/users/organizations/:id/general,/admin/users,/admin/users/:id(breadcrumb mode),/tasks(empty + populated),/task— verify horizontal alignment + tightened gap below tabs + empty-state aligns with/taskSummary by CodeRabbit
New Features
Bug Fixes
Refactor