fix(docker-training): apply design review findings#46
Merged
Conversation
- jochen.html: add missing <section> wrapper (source: shared/sections/jochen.html) - why-docker.html: add source attribution for docker-hub-stats.png; link @benorama credit - cloud-native.html: align Core Technologies icon color to --r-accent-color (was --r-link-color) - container-history.html: replace inline-styled blockquote div with semantic <blockquote> - docker-images.html: replace all off-palette SVG hex colors with CSS custom property references - devops-docker.html: add styled-list class to bare <ul> elements inside .card - docker-kubernetes.html: link @ajeetraina source credit - docker-desktop/networking/volumes/swarm/serverless/kubernetes/monitoring: add loading="lazy" to late-deck images https://claude.ai/code/session_01VVPnYcx8HkWWSqWK7EjFsJ
Preview DeploymentThis PR has been deployed for preview:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR applies design-review improvements across the docker-training presentation and shared speaker slide content, focusing on semantic HTML, consistent theme-driven styling, attribution hygiene, and image loading optimizations.
Changes:
- Standardized styling by aligning icon/SVG colors with the reveal theme’s CSS custom properties and semantics.
- Improved slide markup semantics and consistency (e.g., adding missing
<section>wrapper, replacing a styled<div>quote with<blockquote>, addingstyled-listin cards). - Added/updated image attributions and applied
loading="lazy"to a set of images to reduce upfront load cost.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| shared/sections/jochen.html | Wraps shared speaker slide content in a <section> for consistent external slide inclusion. |
| docker-training/sections/why-docker.html | Improves attribution formatting (linked credit) and adds a missing image source line. |
| docker-training/sections/serverless.html | Adds loading="lazy" to section images. |
| docker-training/sections/monitoring.html | Adds loading="lazy" to the monitoring image. |
| docker-training/sections/docker-volumes.html | Adds loading="lazy" to volume-related images. |
| docker-training/sections/docker-swarm.html | Adds loading="lazy" to swarm images. |
| docker-training/sections/docker-networking.html | Adds loading="lazy" to networking images. |
| docker-training/sections/docker-kubernetes.html | Adds loading="lazy" to images and links the credit handle as an external source. |
| docker-training/sections/docker-images.html | Migrates much of the inline SVG palette to CSS variables and adds lazy-loading to later images. |
| docker-training/sections/docker-desktop.html | Adds loading="lazy" to the architecture image. |
| docker-training/sections/devops-docker.html | Applies styled-list to lists inside .card for consistent rendering. |
| docker-training/sections/container-history.html | Replaces an inline-styled quote container with semantic <blockquote> to leverage theme styling. |
| docker-training/sections/cloud-native.html | Aligns “Core Technologies” icon color usage with --r-accent-color semantics. |
Comments suppressed due to low confidence (1)
docker-training/sections/docker-images.html:95
- This SVG label still hard-codes the heading color (
fill="#1a202c"). Using--r-heading-colorkeeps it consistent with the theme-driven styling used elsewhere in the diagram.
<text x="342" y="187"
font-family="Source Sans Pro,Helvetica,sans-serif"
font-size="12" font-weight="600" fill="#1a202c">Image layers (R/O)</text>
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…CSS vars Replace fill="#1a202c" (4 text labels) and stroke="#1e4555" (fold crease) with var(--r-heading-color) per Copilot review comments on PR #46. https://claude.ai/code/session_01VVPnYcx8HkWWSqWK7EjFsJ
The styled-list spacing caused the Container Separation slide to overflow. These lists sit inside dense .card elements where the extra padding breaks the layout — plain <ul> keeps the content within the slide bounds. https://claude.ai/code/session_01VVPnYcx8HkWWSqWK7EjFsJ
Three corrections based on the docker-training review run: - Invariant #1: flag raw hex matching a token exactly as INFO (for SVG fill/stroke and HTML attrs) — hardcoded values diverge silently when the theme changes even if currently correct - Invariant #2: styled-list caveat — dense cards (icon+heading+p+list+footer) risk overflow; downgrade to INFO and require render-check confirmation before applying the fix - Invariant #7: fix prompt drift — dark endpoint is --r-link-color-dark, not --r-link-color (matches the CSS --r-prog-* token definitions) https://claude.ai/code/session_01VVPnYcx8HkWWSqWK7EjFsJ
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
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.
$(cat <<'EOF'
Summary
jochen.html: Added missing<section>wrapper (fixed in canonical sourceshared/sections/jochen.html)why-docker.html: Added.sourceattribution fordocker-hub-stats.png; linked@benoramacredit with<a>elementcloud-native.html: Changed 7 Core Technologies icons from--r-link-color(gold) →--r-accent-color(green) for consistency with.icon-accentsemanticscontainer-history.html: Replaced inline-styled<div>blockquote with semantic<blockquote>, which also corrects the border colour from gold (--r-link-color) to green (--r-accent-color) via the theme ruledocker-images.html: Replaced all 13 off-palette hex colours in the inline SVG diagram with CSS custom property references (--r-accent-color,--r-muted-color,--r-link-color,--r-card-border,--r-heading-color); addedloading="lazy"to 3 imagesdevops-docker.html: Addedclass="styled-list"to 2 bare<ul>elements inside.carddocker-kubernetes.html: Linked@ajeetrainasource credit; addedloading="lazy"to 4 imagesdocker-networking.html,docker-volumes.html,docker-swarm.html,serverless.html,docker-desktop.html,monitoring.html: Addedloading="lazy"to late-deck images (13 images across 6 files)Test plan
npm run check-render -- docker-training— expect 0 overflow / 0 contrast failures (same as baseline)jochen.htmlspeaker slide renders correctly in all three presentationshttps://claude.ai/code/session_01VVPnYcx8HkWWSqWK7EjFsJ
EOF
)
Generated by Claude Code