Skip to content

Commit e93bdd0

Browse files
committed
feat: declare span/subtitle/borderless as dashboard-owned child tags
Move span, subtitle, break, and borderless tag resolution from global cross-cutting into dashboard-specific context so these tags are only consumed when the field is a dashboard child. Register them as childOwnedPaths in renderer-validation-specs so the ownership model drives unread-tag warnings correctly. Signed-off-by: James Swirhun <james@credibledata.com>
1 parent aac6618 commit e93bdd0

4 files changed

Lines changed: 80 additions & 44 deletions

File tree

packages/malloy-render/src/component/renderer-validation-specs.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ const BUILT_IN_RENDERER_VALIDATION_SPECS: Record<
99
> = {
1010
dashboard: {
1111
renderer: 'dashboard',
12-
childOwnedPaths: [['break']],
12+
childOwnedPaths: [['break'], ['span'], ['subtitle'], ['borderless']],
1313
},
1414
};
1515

packages/malloy-render/src/component/tag-configs.ts

Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -347,38 +347,18 @@ export function resolveBuiltInTags(field: Field): void {
347347
const renderAs = field.renderAs();
348348
const tag = field.tag;
349349

350-
// Cross-cutting: resolve label and subtitle for all fields
350+
// Cross-cutting: resolve label for all fields
351351
const customLabel = tag.text('label');
352352
if (customLabel !== undefined) {
353353
field.setResolvedLabel(customLabel);
354354
}
355355

356-
const subtitle = tag.text('subtitle');
357-
if (subtitle !== undefined) {
358-
field.setResolvedSubtitle(subtitle);
359-
}
360-
361356
// Cross-cutting: resolve column config for all fields
362357
const columnConfig = resolveColumnTags(field);
363358
if (columnConfig) {
364359
field.setColumnConfig(columnConfig);
365360
}
366361

367-
// Cross-cutting: resolve dashboard child-item properties at setup time
368-
// so the dashboard component never reads tags at render time.
369-
const spanVal = tag.numeric('span');
370-
if (spanVal !== undefined) {
371-
field.setResolvedSpan(spanVal);
372-
}
373-
374-
if (tag.has('break')) {
375-
field.setResolvedBreak(true);
376-
}
377-
378-
if (tag.has('borderless')) {
379-
field.setResolvedBorderless(true);
380-
}
381-
382362
// Touch size.height which is read by chart-layout-settings
383363
// (size and size.width are already read by validateFieldTags)
384364
const sizeTag = tag.tag('size');
@@ -405,6 +385,27 @@ export function resolveBuiltInTags(field: Field): void {
405385
break;
406386
case 'dashboard':
407387
field.setTagConfig(resolveDashboardTags(field));
388+
// Resolve dashboard-owned child tags at setup time so the
389+
// dashboard component never reads tags at render time.
390+
if (field.isNest()) {
391+
for (const child of field.fields) {
392+
const childTag = child.tag;
393+
const spanVal = childTag.numeric('span');
394+
if (spanVal !== undefined) {
395+
child.setResolvedSpan(spanVal);
396+
}
397+
const subtitle = childTag.text('subtitle');
398+
if (subtitle !== undefined) {
399+
child.setResolvedSubtitle(subtitle);
400+
}
401+
if (childTag.has('break')) {
402+
child.setResolvedBreak(true);
403+
}
404+
if (childTag.has('borderless')) {
405+
child.setResolvedBorderless(true);
406+
}
407+
}
408+
}
408409
break;
409410
}
410411
}

packages/malloy-render/src/render-field-metadata.ts

Lines changed: 20 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -446,19 +446,7 @@ export class RenderFieldMetadata {
446446
}
447447
}
448448

449-
// --- Dashboard item tags ---
450-
451-
const spanVal = tag.numeric('span');
452-
if (spanVal !== undefined) {
453-
if (!Number.isInteger(spanVal) || spanVal < 1 || spanVal > 12) {
454-
log.error(
455-
`Invalid # span on '${field.name}': expected an integer 1–12, got ${spanVal}. Fix: # span=6.`,
456-
tag.tag('span')
457-
);
458-
}
459-
}
460-
461-
// --- Dashboard view-level tags ---
449+
// --- Dashboard tags (view-level + child item validation) ---
462450

463451
if (tag.has('dashboard') && field.isNest()) {
464452
const dashboardTag = tag.tag('dashboard');
@@ -470,15 +458,6 @@ export class RenderFieldMetadata {
470458
dashboardTag?.tag('columns')
471459
);
472460
}
473-
// Warn if children use # span inside a columns-mode dashboard
474-
for (const child of field.fields) {
475-
if (child.tag.numeric('span') !== undefined) {
476-
log.warn(
477-
`Invalid # span on '${child.name}': span is ignored when parent dashboard '${field.name}' uses columns mode. Fix: remove # span or remove dashboard.columns.`,
478-
child.tag.tag('span')
479-
);
480-
}
481-
}
482461
}
483462
const gapVal = dashboardTag?.numeric('gap');
484463
if (gapVal !== undefined && gapVal < 0) {
@@ -487,6 +466,25 @@ export class RenderFieldMetadata {
487466
dashboardTag?.tag('gap')
488467
);
489468
}
469+
470+
// Validate dashboard-owned child tags
471+
for (const child of field.fields) {
472+
const childSpan = child.tag.numeric('span');
473+
if (childSpan !== undefined) {
474+
if (!Number.isInteger(childSpan) || childSpan < 1 || childSpan > 12) {
475+
log.error(
476+
`Invalid # span on '${child.name}': expected an integer 1–12, got ${childSpan}. Fix: # span=6.`,
477+
child.tag.tag('span')
478+
);
479+
}
480+
if (columnsVal !== undefined) {
481+
log.warn(
482+
`Invalid # span on '${child.name}': span is ignored when parent dashboard '${field.name}' uses columns mode. Fix: remove # span or remove dashboard.columns.`,
483+
child.tag.tag('span')
484+
);
485+
}
486+
}
487+
}
490488
}
491489

492490
// --- Number verbose properties ---

test/src/render/render-validator.spec.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -476,6 +476,27 @@ describe('render tag validation', () => {
476476
expectNoWarnings(logs);
477477
});
478478

479+
it('no warnings for # span, # subtitle, # borderless on dashboard child fields', async () => {
480+
const logs = await getValidationLogs(`
481+
source: s is duckdb.sql("SELECT 1 as a, 2 as b, 3 as c") extend {
482+
# dashboard
483+
view: q is {
484+
group_by: grp is 'all'
485+
aggregate:
486+
# span=6
487+
# subtitle="Total A"
488+
a_total is a.sum()
489+
# borderless
490+
b_total is b.sum()
491+
c_total is c.sum()
492+
}
493+
}
494+
query: q is s -> q
495+
`);
496+
expectNoErrors(logs);
497+
expectNoWarnings(logs);
498+
});
499+
479500
it('no warnings for # tooltip on chart child fields', async () => {
480501
const logs = await getValidationLogs(`
481502
source: s is duckdb.sql("SELECT 'A' as category, 1 as value, 'hello' as note") extend {
@@ -545,6 +566,22 @@ describe('render tag validation', () => {
545566
expect(warnings.some(w => w.message.includes('break'))).toBe(true);
546567
});
547568

569+
it('warns on # span outside dashboard context', async () => {
570+
const logs = await getValidationLogs(`
571+
source: s is duckdb.sql("SELECT 1 as a") extend {
572+
view: q is {
573+
aggregate:
574+
# span=6
575+
a_total is a.sum()
576+
}
577+
}
578+
query: q is s -> q
579+
`);
580+
const warnings = logs.filter(l => l.severity === 'warn');
581+
expect(warnings.length).toBeGreaterThan(0);
582+
expect(warnings.some(w => w.message.includes('span'))).toBe(true);
583+
});
584+
548585
it('warns on # tooltip outside chart context', async () => {
549586
const logs = await getValidationLogs(`
550587
source: s is duckdb.sql("SELECT 'A' as category, 'hello' as note") extend {

0 commit comments

Comments
 (0)