Skip to content

Commit a9d6dea

Browse files
refactor(core): split PageHeader + CorePageHeaderTabs
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.
1 parent ef11b54 commit a9d6dea

10 files changed

Lines changed: 306 additions & 220 deletions

src/modules/admin/tests/admin.layout.unit.tests.js

Lines changed: 39 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -44,27 +44,21 @@ const mountLayout = (configOverrides = {}, routePath = '/admin/users') =>
4444
stubs: {
4545
RouterLink: true,
4646
RouterView: { template: '<div class="router-view-stub" />' },
47-
PageHeader: {
48-
name: 'PageHeader',
49-
props: ['title', 'icon'],
47+
// Single stub for the new bundled header-with-tabs primitive — exposes
48+
// all the props admin.layout passes through, plus the breadcrumb slot.
49+
CorePageHeaderTabs: {
50+
name: 'CorePageHeaderTabs',
51+
props: ['title', 'icon', 'tabs', 'can', 'basePath', 'hideTabs'],
5052
template: `
51-
<div class="page-header-stub" :data-title="title" :data-icon="icon">
53+
<div class="page-header-tabs-stub" :data-title="title" :data-icon="icon" :data-hide-tabs="hideTabs">
5254
<slot name="avatar" />
5355
<slot name="breadcrumb" />
54-
<slot name="tabs" />
5556
<slot name="title" />
5657
<slot name="subtitle" />
5758
<slot name="actions" />
5859
</div>
5960
`,
6061
},
61-
// Stub SurfaceTabBar so we can read the props it receives without rendering full v-tabs.
62-
// name: 'CoreSurfaceTabBar' is required for findComponent({ name: ... }) to work.
63-
CoreSurfaceTabBar: {
64-
name: 'CoreSurfaceTabBar',
65-
props: ['tabs', 'can', 'basePath'],
66-
template: '<div class="surface-tab-bar-stub" :data-tabs-count="tabs?.length || 0" />',
67-
},
6862
},
6963
},
7064
});
@@ -79,24 +73,24 @@ describe('admin.layout', () => {
7973

8074
it('renders the page header', () => {
8175
const wrapper = mountLayout();
82-
expect(wrapper.find('.page-header-stub').exists()).toBe(true);
76+
expect(wrapper.find('.page-header-tabs-stub').exists()).toBe(true);
8377
});
8478

8579
it('renders a <router-view> for nested admin content', () => {
8680
const wrapper = mountLayout();
8781
expect(wrapper.find('.router-view-stub').exists()).toBe(true);
8882
});
8983

90-
it('passes the four built-in tabs to CoreSurfaceTabBar when no extras are configured', () => {
84+
it('passes the four built-in tabs to CorePageHeaderTabs when no extras are configured', () => {
9185
const wrapper = mountLayout();
92-
const bar = wrapper.findComponent({ name: 'CoreSurfaceTabBar' });
93-
expect(bar.exists()).toBe(true);
94-
const tabs = bar.props('tabs');
86+
const headerTabs = wrapper.findComponent({ name: 'CorePageHeaderTabs' });
87+
expect(headerTabs.exists()).toBe(true);
88+
const tabs = headerTabs.props('tabs');
9589
expect(tabs).toHaveLength(4);
9690
expect(tabs.map((t) => t.value)).toEqual(['users', 'organizations', 'readiness', 'activity']);
9791
});
9892

99-
it('passes built-in + extra tabs from config.admin.tabs to CoreSurfaceTabBar', () => {
93+
it('passes built-in + extra tabs from config.admin.tabs to CorePageHeaderTabs', () => {
10094
const wrapper = mountLayout({
10195
admin: {
10296
tabs: [
@@ -105,45 +99,45 @@ describe('admin.layout', () => {
10599
],
106100
},
107101
});
108-
const bar = wrapper.findComponent({ name: 'CoreSurfaceTabBar' });
109-
const tabs = bar.props('tabs');
102+
const headerTabs = wrapper.findComponent({ name: 'CorePageHeaderTabs' });
103+
const tabs = headerTabs.props('tabs');
110104
expect(tabs).toHaveLength(6);
111105
expect(tabs[4].value).toBe('knowledge');
112106
expect(tabs[5].value).toBe('costs');
113107
});
114108

115-
it('passes basePath="/admin" to CoreSurfaceTabBar', () => {
109+
it('passes basePath="/admin" to CorePageHeaderTabs', () => {
116110
const wrapper = mountLayout();
117-
const bar = wrapper.findComponent({ name: 'CoreSurfaceTabBar' });
118-
expect(bar.props('basePath')).toBe('/admin');
111+
const headerTabs = wrapper.findComponent({ name: 'CorePageHeaderTabs' });
112+
expect(headerTabs.props('basePath')).toBe('/admin');
119113
});
120114

121-
it('passes a function `can` predicate to CoreSurfaceTabBar', () => {
115+
it('passes a function `can` predicate to CorePageHeaderTabs', () => {
122116
const wrapper = mountLayout();
123-
const bar = wrapper.findComponent({ name: 'CoreSurfaceTabBar' });
124-
expect(typeof bar.props('can')).toBe('function');
117+
const headerTabs = wrapper.findComponent({ name: 'CorePageHeaderTabs' });
118+
expect(typeof headerTabs.props('can')).toBe('function');
125119
});
126120

127-
it('gracefully handles non-array admin.tabs (CoreSurfaceTabBar receives only the built-in 4)', () => {
121+
it('gracefully handles non-array admin.tabs (CorePageHeaderTabs receives only the built-in 4)', () => {
128122
const wrapper = mountLayout({ admin: { tabs: 'invalid' } });
129-
const bar = wrapper.findComponent({ name: 'CoreSurfaceTabBar' });
130-
expect(bar.props('tabs')).toHaveLength(4);
123+
const headerTabs = wrapper.findComponent({ name: 'CorePageHeaderTabs' });
124+
expect(headerTabs.props('tabs')).toHaveLength(4);
131125
});
132126

133127
it('renders the error banner at the TOP of the layout, before the header', async () => {
134128
adminStoreState.error = 'Boom';
135129
const wrapper = mountLayout();
136130
await wrapper.vm.$nextTick();
137131
const html = wrapper.html();
138-
expect(html.indexOf('Boom')).toBeLessThan(html.indexOf('page-header-stub'));
132+
expect(html.indexOf('Boom')).toBeLessThan(html.indexOf('page-header-tabs-stub'));
139133
});
140134

141135
it('renders the mailer warning at the TOP when serverConfig.mail.configured is false', async () => {
142136
authStoreState.serverConfig = { mail: { configured: false } };
143137
const wrapper = mountLayout();
144138
await wrapper.vm.$nextTick();
145139
const html = wrapper.html();
146-
expect(html.indexOf('No mailer configured')).toBeLessThan(html.indexOf('page-header-stub'));
140+
expect(html.indexOf('No mailer configured')).toBeLessThan(html.indexOf('page-header-tabs-stub'));
147141
});
148142

149143
it('does NOT render the mailer warning when mail is configured', () => {
@@ -152,16 +146,6 @@ describe('admin.layout', () => {
152146
expect(wrapper.html()).not.toContain('No mailer configured');
153147
});
154148

155-
it('renders <CoreSurfaceTabBar> as a SIBLING of <PageHeader> (not inside its #tabs slot)', () => {
156-
const wrapper = mountLayout();
157-
const pageHeaderEl = wrapper.find('.page-header-stub');
158-
const surfaceTabBarEl = wrapper.find('.surface-tab-bar-stub');
159-
expect(pageHeaderEl.exists()).toBe(true);
160-
expect(surfaceTabBarEl.exists()).toBe(true);
161-
// Sibling layout: the tab-bar element is NOT inside the page-header-stub element.
162-
expect(pageHeaderEl.element.contains(surfaceTabBarEl.element)).toBe(false);
163-
});
164-
165149
it('renders <router-view> OUTSIDE the layout <v-container>', () => {
166150
const wrapper = mountLayout();
167151
const layoutContainer = wrapper.find('.v-container');
@@ -171,32 +155,34 @@ describe('admin.layout', () => {
171155
expect(layoutContainer.element.contains(routerViewEl.element)).toBe(false);
172156
});
173157

174-
it('PageHeader receives title="Admin" + icon="fa-solid fa-user-tie" in list mode', () => {
158+
it('CorePageHeaderTabs receives title="Admin" + icon="fa-solid fa-user-tie" in list mode', () => {
175159
const wrapper = mountLayout();
176-
const ph = wrapper.findComponent({ name: 'PageHeader' });
177-
expect(ph.props('title')).toBe('Admin');
178-
expect(ph.props('icon')).toBe('fa-solid fa-user-tie');
160+
const headerTabs = wrapper.findComponent({ name: 'CorePageHeaderTabs' });
161+
expect(headerTabs.props('title')).toBe('Admin');
162+
expect(headerTabs.props('icon')).toBe('fa-solid fa-user-tie');
179163
});
180164

181-
it('PageHeader receives title="" + icon stays in breadcrumb mode', async () => {
165+
it('CorePageHeaderTabs receives title="" + icon stays in breadcrumb mode', async () => {
182166
adminStoreState.currentBreadcrumb = { title: 'Jane Doe' };
183167
const wrapper = mountLayout({}, '/admin/users/u1');
184168
await wrapper.vm.$nextTick();
185-
const ph = wrapper.findComponent({ name: 'PageHeader' });
186-
expect(ph.props('title')).toBe('');
187-
expect(ph.props('icon')).toBe('fa-solid fa-user-tie');
169+
const headerTabs = wrapper.findComponent({ name: 'CorePageHeaderTabs' });
170+
expect(headerTabs.props('title')).toBe('');
171+
expect(headerTabs.props('icon')).toBe('fa-solid fa-user-tie');
188172
});
189173

190-
it('does NOT render <CoreSurfaceTabBar> when currentBreadcrumb is set (detail mode)', async () => {
174+
it('passes hideTabs=true when currentBreadcrumb is set (detail mode)', async () => {
191175
adminStoreState.currentBreadcrumb = { title: 'Jane Doe' };
192176
const wrapper = mountLayout({}, '/admin/users/u1');
193177
await wrapper.vm.$nextTick();
194-
expect(wrapper.findComponent({ name: 'CoreSurfaceTabBar' }).exists()).toBe(false);
178+
const headerTabs = wrapper.findComponent({ name: 'CorePageHeaderTabs' });
179+
expect(headerTabs.props('hideTabs')).toBe(true);
195180
});
196181

197-
it('renders <CoreSurfaceTabBar> when currentBreadcrumb is null (list mode)', () => {
182+
it('passes hideTabs=false when currentBreadcrumb is null (list mode)', () => {
198183
const wrapper = mountLayout();
199-
expect(wrapper.findComponent({ name: 'CoreSurfaceTabBar' }).exists()).toBe(true);
184+
const headerTabs = wrapper.findComponent({ name: 'CorePageHeaderTabs' });
185+
expect(headerTabs.props('hideTabs')).toBe(false);
200186
});
201187

202188
it('renders the breadcrumb when useAdminStore().currentBreadcrumb is set', async () => {

src/modules/admin/views/admin.layout.vue

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -27,22 +27,20 @@
2727
</span>
2828
</v-alert>
2929

30-
<PageHeader
30+
<CorePageHeaderTabs
3131
icon="fa-solid fa-user-tie"
3232
:title="currentBreadcrumb ? '' : 'Admin'"
33+
:tabs="allTabs"
34+
:can="adminCan"
35+
:base-path="basePath"
36+
:hide-tabs="!!currentBreadcrumb"
3337
>
3438
<template v-if="currentBreadcrumb" #breadcrumb>
3539
<router-link to="/admin" class="text-medium-emphasis text-decoration-none">Admin</router-link>
3640
<v-icon icon="fa-solid fa-chevron-right" size="x-small" class="mx-2 text-medium-emphasis"></v-icon>
3741
<span :class="currentBreadcrumb.titleClass || ''">{{ currentBreadcrumb.title }}</span>
3842
</template>
39-
</PageHeader>
40-
<CoreSurfaceTabBar
41-
v-if="!currentBreadcrumb"
42-
:tabs="allTabs"
43-
:can="adminCan"
44-
:base-path="basePath"
45-
/>
43+
</CorePageHeaderTabs>
4644
</v-container>
4745

4846
<router-view />
@@ -51,8 +49,7 @@
5149
/**
5250
* Module dependencies.
5351
*/
54-
import PageHeader from '../../core/components/core.pageHeader.component.vue';
55-
import CoreSurfaceTabBar from '../../core/components/core.surfaceTabBar.component.vue';
52+
import CorePageHeaderTabs from '../../core/components/core.pageHeaderTabs.component.vue';
5653
import { ability } from '../../../lib/helpers/ability';
5754
import { useAdminStore } from '../stores/admin.store';
5855
import { useAuthStore } from '../../auth/stores/auth.store';
@@ -85,7 +82,7 @@ const BUILT_IN_TABS = Object.freeze([
8582
*/
8683
export default {
8784
name: 'AdminLayout',
88-
components: { PageHeader, CoreSurfaceTabBar },
85+
components: { CorePageHeaderTabs },
8986
computed: {
9087
/**
9188
* @desc Base path for admin tabs (where CoreSurfaceTabBar resolves relative routes).

src/modules/core/components/core.pageHeader.component.vue

Lines changed: 38 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1,50 +1,44 @@
11
<template>
22
<v-row class="core-page-header mx-4 my-1 flex-nowrap" align="center">
3-
<template v-if="$slots.tabs">
4-
<!-- Tabs mode: tabs replace icon + title -->
5-
<slot name="tabs"></slot>
6-
</template>
7-
<template v-else>
8-
<!-- Standard mode: icon + (breadcrumb OR title) -->
9-
<slot v-if="!$vuetify.display.mobile" name="avatar">
10-
<v-avatar v-if="icon" :color="avatarColor" size="40">
11-
<v-icon :icon="icon" size="small" color="white"></v-icon>
12-
</v-avatar>
13-
</slot>
3+
<!-- Standard mode: icon + (breadcrumb OR title) -->
4+
<slot v-if="!$vuetify.display.mobile" name="avatar">
5+
<v-avatar v-if="icon" :color="avatarColor" size="40">
6+
<v-icon :icon="icon" size="small" color="white"></v-icon>
7+
</v-avatar>
8+
</slot>
9+
<div
10+
class="core-page-header__content"
11+
:class="[
12+
$vuetify.display.mobile ? 'ml-6' : '',
13+
!$vuetify.display.mobile && ($slots.avatar || icon) ? 'ml-2' : '',
14+
]"
15+
>
16+
<!-- Breadcrumb takes precedence over title when provided -->
1417
<div
15-
class="core-page-header__content"
16-
:class="[
17-
$vuetify.display.mobile ? 'ml-6' : '',
18-
!$vuetify.display.mobile && ($slots.avatar || icon) ? 'ml-2' : '',
19-
]"
18+
v-if="$slots.breadcrumb"
19+
class="core-page-header__breadcrumb text-title-large font-weight-medium text-truncate"
20+
:class="titleClass"
21+
style="line-height: 1;"
2022
>
21-
<!-- Breadcrumb takes precedence over title when provided -->
22-
<div
23-
v-if="$slots.breadcrumb"
24-
class="core-page-header__breadcrumb text-title-large font-weight-medium text-truncate"
23+
<slot name="breadcrumb"></slot>
24+
</div>
25+
<template v-else>
26+
<h2
27+
class="core-page-header__title text-title-large font-weight-medium text-truncate"
2528
:class="titleClass"
2629
style="line-height: 1;"
2730
>
28-
<slot name="breadcrumb"></slot>
29-
</div>
30-
<template v-else>
31-
<h2
32-
class="core-page-header__title text-title-large font-weight-medium text-truncate"
33-
:class="titleClass"
34-
style="line-height: 1;"
35-
>
36-
<slot name="title">{{ title }}</slot>
37-
</h2>
38-
<p
39-
v-if="subtitle || $slots.subtitle"
40-
class="core-page-header__subtitle text-body-medium text-medium-emphasis text-truncate"
41-
style="margin: 0; line-height: 1;"
42-
>
43-
<slot name="subtitle">{{ subtitle }}</slot>
44-
</p>
45-
</template>
46-
</div>
47-
</template>
31+
<slot name="title">{{ title }}</slot>
32+
</h2>
33+
<p
34+
v-if="subtitle || $slots.subtitle"
35+
class="core-page-header__subtitle text-body-medium text-medium-emphasis text-truncate"
36+
style="margin: 0; line-height: 1;"
37+
>
38+
<slot name="subtitle">{{ subtitle }}</slot>
39+
</p>
40+
</template>
41+
</div>
4842
<v-spacer></v-spacer>
4943
<!-- Actions slot — wrapped in a flex cluster (flex-wrap = safety net for extreme viewports) -->
5044
<div v-if="$slots.actions" class="core-page-header__actions d-flex align-center flex-wrap">
@@ -82,19 +76,14 @@ export default {
8276
</script>
8377

8478
<style scoped>
85-
/*
86-
* Canonical 56px height — keeps list view (title) and detail view (breadcrumb)
87-
* visually aligned so route transitions don't jump.
88-
*/
89-
.core-page-header {
90-
min-height: 56px;
91-
max-height: 56px;
92-
}
93-
9479
/*
9580
* flex: 1 + min-width: 0 lets the title/breadcrumb container shrink past its
9681
* intrinsic width when content is long, so the truncate kicks in instead of
9782
* pushing the actions cluster off-screen.
83+
*
84+
* NOTE: PageHeader is content-sized by default. Tabbed surfaces that need
85+
* a 56px rhythm for title↔breadcrumb route transitions use CorePageHeaderTabs,
86+
* which enforces the height externally.
9887
*/
9988
.core-page-header__content {
10089
flex: 1 1 0;

0 commit comments

Comments
 (0)