Skip to content

Workshop: CSS-driven journey-tag visibility for rendered step content #46720

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pelikhan merged 3 commits into from
Jul 20, 2026
+192 −1
Merged

Workshop: CSS-driven journey-tag visibility for rendered step content #46720

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9ffe035
Add CSS-driven journey section visibility in workshop content
Copilot Jul 20, 2026
f2ea7b0
Refine journey block parsing for workshop visibility
Copilot Jul 20, 2026
b0ce6b6
Fix uniqueItems ReferenceError in inline script; add journey block vi…
Copilot Jul 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 52 additions & 1 deletion docs/src/components/workshop/WorkshopExperience.astro
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,8 +229,41 @@ function rewriteGfmTaskLists(html: string) {
return result;
}

function normalizeJourneyToken(value: string) {
return value.trim().toLowerCase().replace(/[^a-z0-9-]/gu, '');
}

const journeyBlockPattern = /<!--\s*journey:\s*([a-z0-9,\s-]+)\s*-->([\s\S]*?)<!--\s*\/journey\s*-->/giu;
const journeyOpenPattern = /<!--\s*journey:\s*[a-z0-9,\s-]+\s*-->/giu;
const journeyClosePattern = /<!--\s*\/journey\s*-->/giu;

function parseJourneyTokens(value: string) {
return uniqueItems(
value
.split(',')
.map(normalizeJourneyToken)
.filter(Boolean),
);
}

function rewriteJourneyBlocks(html: string) {
const wrapped = html.replace(
journeyBlockPattern,
(_match, journeyCsv, body) => {
const journeys = parseJourneyTokens(String(journeyCsv));
if (journeys.length === 0) return body;
const classes = ['aw-workshop-journey-block', ...journeys.map((journey) => `aw-workshop-journey-block-${journey}`)];
return `<div class="${classes.join(' ')}">${body}</div>`;
},
);

return wrapped
.replace(journeyOpenPattern, '')
.replace(journeyClosePattern, '');
}

function rewriteWorkshopHtml(html: string) {
return transformTables(rewriteGfmTaskLists(rewriteGfmAlerts(sanitizeWorkshopHtml(html)))
return transformTables(rewriteJourneyBlocks(rewriteGfmTaskLists(rewriteGfmAlerts(sanitizeWorkshopHtml(html))))

@github-actions github-actions Bot Jul 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/codebase-design] The closing paren for rewriteJourneyBlocks(...) is mismatched — transformTables(rewriteJourneyBlocks(rewriteGfmTaskLists(rewriteGfmAlerts(sanitizeWorkshopHtml(html)))) has four closing parens for five opening parens. The .replace(...) chain that follows is chained on rewriteJourneyBlocks's result, not on the transformTables call, meaning transformTables receives only the raw string without heading/link/image rewrites.

💡 Expected parenthesisation

If this compiles, verify whether the heading level-bumps, local-link rewriting, and asset-URL resolution are intentionally applied before transformTables, or whether there is a missing closing paren that should wrap the entire chain.

@copilot please address this.


.replace(/<(\/?)h([1-4])\b/gu, (_match, slash, level) => {
return `<${slash}h${Math.min(Number(level) + 1, 4)}`;
Expand Down Expand Up @@ -362,6 +395,8 @@ const workshopSteps: WorkshopStep[] = await Promise.all(workshopEntries.map(asyn
const initialFlow = buildWorkshopFlow(workshopDefaults.journeyId, workshopDefaults.scenarioId);
const initialStepKey = initialFlow[0] ?? workshopSteps[0]?.key ?? '';
const initialStep = workshopSteps.find((item) => item.key === initialStepKey) ?? workshopSteps[0];
const initialJourney = workshopJourneys.find((item) => item.id === workshopDefaults.journeyId) ?? workshopJourneys[0];
const initialVisibleJourneyIds = uniqueItems(['all', ...(initialJourney?.contentJourneyIds ?? [])]);

// Precompute step counts for entry path cards using the default scenario (estimate shown before scenario is selected).
const entryPathStepCounts: Record<string, number> = Object.fromEntries(
Expand All @@ -388,6 +423,7 @@ const scenarioStepCountsByJourney: Record<string, Record<string, number>> = Obje
<section
class="aw-workshop"
data-workshop-root
data-workshop-visible-journeys={initialVisibleJourneyIds.join(' ')}
data-initial-step={initialStepKey}
>
<div class="aw-workshop-setup" data-workshop-setup>
Expand Down Expand Up @@ -840,6 +876,8 @@ const scenarioStepCountsByJourney: Record<string, Record<string, number>> = Obje
const journey = manifest.journeys.find((item) => item.id === state.journeyId) || manifest.journeys[0];
const scenario = manifest.scenarios.find((item) => item.id === state.scenarioId) || manifest.scenarios[0];
const stepIndex = visibleFlow.indexOf(activeStep.key);
const visibleJourneyIds = ['all', ...((journey && journey.contentJourneyIds) || [])];
root.dataset.workshopVisibleJourneys = [...new Set(visibleJourneyIds)].join(' ');
if (tunnelPanel) {
tunnelPanel.hidden = state.journeyId !== 'vscode' || setupStep !== 'scenario';
}
Expand Down Expand Up @@ -1307,6 +1345,19 @@ const scenarioStepCountsByJourney: Record<string, Record<string, number>> = Obje
min-height: 22rem;
}

.aw-workshop-step-content :global(.aw-workshop-journey-block) {
display: none;
}

.aw-workshop-step-content :global(.aw-workshop-journey-block.aw-workshop-journey-block-all),
.aw-workshop[data-workshop-visible-journeys~='ui'] .aw-workshop-step-content :global(.aw-workshop-journey-block.aw-workshop-journey-block-ui),

@github-actions github-actions Bot Jul 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/codebase-design] The CSS hardcodes a closed list of journey tokens (ui, terminal, local, codespace, copilot). Any new journey added to the manifest will be invisible without a corresponding CSS rule update — a silent failure mode.

💡 Data-driven alternative

The data-workshop-visible-journeys attribute already holds the active token list. A single attribute selector can match any token dynamically without enumerating every journey:

/* hide all journey blocks by default */
.aw-workshop-step-content :global(.aw-workshop-journey-block) {
  display: none;
}

/* JS sets: root.dataset.workshopActiveJourney = "ui" (single token)
   then CSS can match generically -- requires a different data model */

Alternatively, document in a code comment that new journeys require a corresponding CSS rule, so it is not accidentally missed.

@copilot please address this.

.aw-workshop[data-workshop-visible-journeys~='terminal'] .aw-workshop-step-content :global(.aw-workshop-journey-block.aw-workshop-journey-block-terminal),
.aw-workshop[data-workshop-visible-journeys~='local'] .aw-workshop-step-content :global(.aw-workshop-journey-block.aw-workshop-journey-block-local),
.aw-workshop[data-workshop-visible-journeys~='codespace'] .aw-workshop-step-content :global(.aw-workshop-journey-block.aw-workshop-journey-block-codespace),
.aw-workshop[data-workshop-visible-journeys~='copilot'] .aw-workshop-step-content :global(.aw-workshop-journey-block.aw-workshop-journey-block-copilot) {

@github-actions github-actions Bot Jul 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CSS visibility selectors hardcode the known journey names (ui, terminal, local, codespace, copilot). Any workshop manifest that introduces a new contentJourneyId value will silently keep those blocks hidden — the data-workshop-visible-journeys attribute will be updated correctly but no CSS rule will match.

Consider generating the selectors dynamically in JavaScript (e.g., adding/removing a is-visible class on the journey blocks themselves) rather than relying on a hardcoded CSS list, or at minimum add a comment here listing the known values so reviewers know to update this selector table when a new journey type is introduced.

@copilot please address this.

display: contents;

@github-actions github-actions Bot Jul 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/codebase-design] display: contents is intentional for flow preservation, but it has a known browser accessibility-tree bug: the wrapper <div> is removed from the a11y tree, which can break landmark/list semantics for screen readers if journey blocks wrap <li>, <td>, or interactive elements.

💡 Safer alternative

Use display: block / display: none (the default already hides with display: none; the reveal just needs display: block). Since the <div> is a neutral wrapper, display: block is safe and avoids the a11y issue:

.aw-workshop-step-content :global(.aw-workshop-journey-block.aw-workshop-journey-block-all),
/* ... */
{
  display: block; /* was: display: contents */
}

display: contents would only be needed if journey blocks were placed inside flex/grid containers where an extra wrapper would break layout — verify that is actually the case before keeping it.

@copilot please address this.

}

.aw-workshop-step-content :global(h2),
.aw-workshop-step-content :global(h3),
.aw-workshop-step-content :global(h4) {
Expand Down
140 changes: 140 additions & 0 deletions docs/tests/workshop.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -583,3 +583,143 @@ test.describe('Workshop flow filtering: Copilot scenario-d substitution', () =>
expect(keys).not.toContain('11c-build-pr-reviewer-ui');
});
});

// ---------------------------------------------------------------------------
// Journey block visibility tests — verify that CSS-driven journey section
// visibility works correctly: marker rewriting, attribute state, and
// visibility transitions when switching paths.
// ---------------------------------------------------------------------------

test.describe('Workshop journey block visibility', () => {
test('data-workshop-visible-journeys contains "all" and journey-specific IDs after starting the tutorial', async ({ page }) => {
// ui-learner maps to the github journey, which has contentJourneyIds: ['ui']
await startWorkshop(page);

const visibleJourneys = await page.evaluate(() =>
document.querySelector('[data-workshop-root]')?.getAttribute('data-workshop-visible-journeys') ?? '',
);
// 'all' is always included; 'ui' comes from the github journey's contentJourneyIds
expect(visibleJourneys.split(' ')).toContain('all');
expect(visibleJourneys.split(' ')).toContain('ui');
});

test('data-workshop-visible-journeys updates when switching to a different entry path', async ({ page }) => {
// Start with ui-learner (github journey → contentJourneyIds: ['ui'])
await startWorkshop(page);

const initialJourneys = await page.evaluate(() =>
document.querySelector('[data-workshop-root]')?.getAttribute('data-workshop-visible-journeys') ?? '',
);
expect(initialJourneys.split(' ')).toContain('ui');

// Switch to the terminal path (contentJourneyIds: ['terminal', 'local'])
await page.getByRole('button', { name: /Change route/i }).click();
await page.locator('[data-workshop-entry-path="cli-user"]').click();
await page.locator('[data-workshop-scenario="daily-status"]').click();
await expect(page.locator('[data-workshop-tutorial]')).toBeVisible();

const updatedJourneys = await page.evaluate(() =>
document.querySelector('[data-workshop-root]')?.getAttribute('data-workshop-visible-journeys') ?? '',
);
expect(updatedJourneys.split(' ')).toContain('all');
expect(updatedJourneys.split(' ')).toContain('terminal');
// 'ui' should no longer be listed (terminal journey does not map to ui content)
expect(updatedJourneys.split(' ')).not.toContain('ui');
});

test('no raw journey comment markers survive HTML rewriting in step data', async ({ page }) => {
await startWorkshop(page);

// Any <!-- journey: ... --> or <!-- /journey --> remaining in rendered HTML
// means rewriteJourneyBlocks did not run or failed. Raw markers must never
// appear in the embedded step-data JSON.
const hasRawMarkers = await page.evaluate(() => {
const node = document.getElementById('aw-workshop-step-data');
if (!node) return false;
const steps = JSON.parse(node.textContent?.trim() || '[]') as Array<{ html: string }>;
return steps.some((s) => /<!--\s*\/?journey[:\s]/i.test(s.html));
});

expect(hasRawMarkers).toBe(false);
});

test('journey blocks in step data have base class and one class per comma-separated token', async ({ page }) => {
await startWorkshop(page);

// If any step HTML contains journey blocks, verify each wrapper carries the
// base class plus a distinct class per token. Passes vacuously when the
// current workshop build has no journey-tagged sections.
const result = await page.evaluate(() => {
const node = document.getElementById('aw-workshop-step-data');
if (!node) return { hasJourneyBlocks: false, allValid: true };
const steps = JSON.parse(node.textContent?.trim() || '[]') as Array<{ html: string }>;

const wrapperPattern = /class="([^"]*aw-workshop-journey-block[^"]*)"/g;
let hasJourneyBlocks = false;
let allValid = true;

for (const step of steps) {
for (const match of step.html.matchAll(wrapperPattern)) {
hasJourneyBlocks = true;
const classList = match[1].split(/\s+/);
// Must have the base class
if (!classList.includes('aw-workshop-journey-block')) {
allValid = false;
}
// Must have at least one journey-specific class
if (!classList.some((c) => c.startsWith('aw-workshop-journey-block-') && c !== 'aw-workshop-journey-block')) {
allValid = false;
}
}
}

return { hasJourneyBlocks, allValid };
});

expect(result.allValid).toBe(true);
});

test('journey blocks are hidden by default and revealed via CSS when the active journey matches', async ({ page }) => {
await startWorkshop(page);

// Verify CSS-driven visibility using an injected probe element.
// The github journey (ui-learner path) maps to contentJourneyIds: ['ui'],
// so a block tagged 'ui' should be visible and one tagged 'terminal' should be hidden.
const visibility = await page.evaluate(() => {
const stepContent = document.querySelector('[data-workshop-step-content]') as HTMLElement | null;
if (!stepContent) return null;

const uiBlock = document.createElement('div');
uiBlock.className = 'aw-workshop-journey-block aw-workshop-journey-block-ui';
uiBlock.setAttribute('data-test-probe', 'ui');

const terminalBlock = document.createElement('div');
terminalBlock.className = 'aw-workshop-journey-block aw-workshop-journey-block-terminal';
terminalBlock.setAttribute('data-test-probe', 'terminal');

const allBlock = document.createElement('div');
allBlock.className = 'aw-workshop-journey-block aw-workshop-journey-block-all';
allBlock.setAttribute('data-test-probe', 'all');

stepContent.appendChild(uiBlock);
stepContent.appendChild(terminalBlock);
stepContent.appendChild(allBlock);

return {
uiDisplay: window.getComputedStyle(uiBlock).display,
terminalDisplay: window.getComputedStyle(terminalBlock).display,
allDisplay: window.getComputedStyle(allBlock).display,
};
});

expect(visibility).not.toBeNull();
if (visibility) {
// 'all' blocks are always visible regardless of journey
expect(visibility.allDisplay).toBe('contents');
// 'ui' block is visible because the github journey includes 'ui' in contentJourneyIds
expect(visibility.uiDisplay).toBe('contents');
// 'terminal' block is hidden because the github journey does not include 'terminal'
expect(visibility.terminalDisplay).toBe('none');
}
});
});
Loading