Add DESIGN.md, fix SCSS entry-point import order, correct docs examples - #245
Open
daphneslootmans wants to merge 8 commits into
Open
Add DESIGN.md, fix SCSS entry-point import order, correct docs examples#245daphneslootmans wants to merge 8 commits into
daphneslootmans wants to merge 8 commits into
Conversation
…oints error.scss and mail.scss imported vendor/twbs/bootstrap/scss/variables before bootstrap-variables. Both sides use !default, so the vendor value won and every override in bootstrap-variables was silently inert: error pages and emails rendered with stock Bootstrap tokens instead of the bundle's. Reordering restores them. Verified by compiling against Bootstrap 5.3.3: error.css now emits --bs-border-radius 0.25rem (was the stock 0.375rem), --bs-gray-900 #2d2f35 (was #212529), --bs-body-bg #f8f9fa (was #fff), pill buttons and Lato; mail.css picks up $dark #383a43 for body and subheader text. error.scss also now imports bootstrap-variables-dark. base_error.html.twig includes settheme.html.twig, so error pages do get data-bs-theme, but the dark custom properties were never emitted and base/_error.scss reads var(--error-bg) and var(--error-content-bg). Dark error pages used light colours. mail.scss deliberately omits the dark file: Foundation for Emails has no colour modes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Single reference at the repo root, in the designmd.ai format, describing the
shared admin theme every project inherits. Linked from docs/index.md.
Values were reverse-engineered from assets/scss and templates, then diffed
against stock Bootstrap 5.3.3 so only deliberate decisions are recorded. Covers
colours (including the non-stock ninth "white" theme colour), the compressed
and RFS-fluid type scale, spacing, the sidebar/topbar shell, elevation, the
pre-5.3 focus model, a full 37-entry partial inventory matching _imports.scss,
dark mode, and the conventions for changing any of it.
Also documents the card-based page composition rule: everything inside
{% block main %} sits in a card, one card per section or per overview item,
separated with mb-3 or a gy-3 row, and tables keep their card-body.
Eleven open questions are recorded rather than fixed, including the dead
$white: #e1e1e1 in the dark variables file, the undeclared --user-bg consumed
by .user-nav, and twbs/bootstrap being an undeclared peer dependency.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The three page examples extended 'layout.html.twig' and filled
{% block content %}. Neither exists: layout.html.twig is absent from
application-skeleton and every sibling project, and the bundle's
base.html.twig defines main, not content. Only Mail/base.html.twig defines a
content block. Corrected to extend 'base.html.twig' and fill {% block main %},
with a note recording the real chain so the error does not recur.
Examples now follow the card composition rule: table and pagination in one
card > card-body, each form in its own card. Tables keep the card-body because
a list section usually carries a title or intro text alongside the table.
Also fixes four twig-cs-fixer errors in the delete-form example: the padded
hash literal and a missing trailing comma on the stimulus_controller parameter
list.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every twig snippet in docs/ now lints clean (16 snippets, 0 errors), verified
by extracting each fenced block to a real .twig file and running the project's
twig-cs-fixer config.
- stimulus.md: unpadded the hash literal in the delete-form example and added
trailing commas to both stimulus_controller parameter lists
- pagination.md: {% include %} tag replaced with the include() function
- language-switch.md: dropped unnecessary quoting of the _locale hash key
- mails.md: removed trailing whitespace on two lines
The pagination.md and language-switch.md edits change code rather than
whitespace; both are exact Twig equivalents with no behaviour change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Reviewer's GuideThe PR fixes inert theme overrides in error and mail SCSS entry points—also restoring dark error-page tokens—while adding a comprehensive DESIGN.md reference, correcting CRUD template examples to match the bundle's inheritance and card conventions, and making documentation Twig snippets lint clean. Flow diagram for corrected SCSS entry-point import orderflowchart LR
F[Bootstrap functions] --> O[Bundle variable overrides]
O --> D[Dark-mode overrides]
D --> V[Bootstrap variables]
V --> VD[Bootstrap dark variables]
VD --> I[Selective Bootstrap imports]
I --> C[Bundle components and layouts]
C --> CSS[Compiled theme CSS]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
$table-striped-bg was var(--bs-emphasis-color), a solid colour, where stock is
rgba(var(--bs-emphasis-color-rgb), $table-striped-bg-factor). The factor was
still declared at 0.05 directly above it but never used, so the rgba wrapper
and the -rgb suffix had been dropped.
--bs-emphasis-color resolves to #000 in light mode and #fff in dark, so plain
.table-striped rendered solid black odd rows (solid white in dark) instead of a
5% tint. Only the .table-{color} variants computed sane values. docs/crud.md
prescribes <table class="table">, so this was one class away from being hit.
Compiled output now emits rgba(var(--bs-emphasis-color-rgb), 0.05), and the
diff against the previous build is that single line.
DESIGN.md updates alongside it:
- drops the $table-striped-bg open question, now fixed, and records the
corrected behaviour in the Tables section
- rewords the $variable-prefix entry. It is not a defect: the fork defines
$variable-prefix: bs- at :392 with $prefix: $variable-prefix on the next
line, and the compiled CSS emits var(--bs-font-sans-serif) correctly. It is
a deprecated name still wired up, worth renaming before an upstream release
drops the alias
- adds an open question for $table-hover-bg and $table-active-bg, which use
rgba($black, ...) instead of the emphasis-color-rgb var and so darken in
dark mode rather than lightening
- removes all 17 em dashes to match the house style and the rest of docs/
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two defects found by rendering the theme in a real project.
Table striping, hover and active never worked, in either theme.
Three causes stacked. $table-accent-bg was $white where stock is transparent.
Bootstrap 5.3 paints every cell with
box-shadow: inset 0 0 0 9999px var(--bs-table-bg-state, var(--bs-table-bg-type,
var(--bs-table-accent-bg))), so that white fallback flooded all cells. The
flood was then masked with a blanket
> :not(caption) > * > * { box-shadow: none; }
in components/_tables.scss. That shadow is the row-state mechanism itself, so
clearing it disabled striping, hover and active along with the flood.
$table-accent-bg goes back to transparent and the blanket reset is removed.
The thead reset stays, so headers remain flat. Combined with the earlier
$table-striped-bg fix, odd rows now render a 5% tint that follows the theme.
This also explains $table-active-bg-factor: 0.75 against a stock 0.1. It was
never visible. Now that active rows paint, it shows as a near-black overlay.
Left as an open question rather than guessed at.
Focus was invisible in dark mode.
$input-focus-border-color is a hardcoded #495057 with an rgba($black, .17)
glow and there was no dark-mode override at all. Against the #383a43 dark
surface both disappear, failing WCAG 2.4.7. components/_forms.scss now
inverts the pair under color-mode(dark): a $gray-400 #ced4da border with an
rgba($white, .25) glow.
Both fixes change rendering for every project on this bundle, tables most of
all, since striping and hover have not worked until now.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
$table-hover-bg is rgba($black, 0.075) rather than stock's
rgba(var(--#{$prefix}emphasis-color-rgb), $table-hover-bg-factor), so hover
darkens in dark mode instead of lightening and reads faint on a dark row.
Reviewed and kept deliberately.
Moves it out of the open questions list into the Tables section, where it
belongs as a documented decision with a note not to "fix" it to the stock
expression unasked. Open questions now holds only genuinely open items.
$table-active-bg shares the hardcoded form and its 0.75 factor against a stock
0.1 stays open, tracked separately from this hover decision.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
$table-hover-bg and $table-active-bg were rgba($black, ...) where stock uses
rgba(var(--#{$prefix}emphasis-color-rgb), ...), so both darkened in dark mode
instead of lightening: hover was near-invisible on a dark row.
$table-active-bg-factor moves from 0.75 to the stock 0.1 as part of the same
change, not as a separate cleanup. Restoring the theme-aware form while leaving
0.75 would have made it worse, turning active rows into a 75% white wash in
dark mode instead of a 75% black one in light.
All three row states are now stock and in a sensible order of prominence:
striped 5%, hover 7.5%, active 10%, each a tint of --bs-emphasis-color-rgb.
Verified in a real project build, with no hardcoded rgba(0, 0, 0, ...) left in
any of the three.
These values were unobservable until the row-state box-shadow reset was removed
earlier in this branch, which is why the 0.75 had gone unnoticed.
DESIGN.md drops the accepted-deviation note on hover and the open question on
active, since neither describes the code any more, and records the row-state
tints as stock.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
| application's vendor tree. `twbs/bootstrap` and `twbs/bootstrap-icons` are not in this bundle's | ||
| `composer.json`; the application supplies them. | ||
|
|
||
| Everything below is a deliberate deviation from stock Bootstrap 5.3.3. Stock values are not |
Member
There was a problem hiding this comment.
Nuttig om zo heel specifiek 5.3.3 te vermelden. Ik kan me inbeelden dat dat snel kan veranderen. Zou enkel Bootstrap 5 vermelden.
| `$secondary: $gray-600` `#6c757d`, `$dark: $gray-800` `#383a43` (stock points at `$gray-900`). | ||
| The bundle ships a neutral base and expects the application to override `$primary`. | ||
|
|
||
| `$theme-colors` carries a **ninth, non-stock entry: `"white": $white`**. Every |
Member
There was a problem hiding this comment.
Is dat effectief in gebruik? Want anders lijkt me dat heel veel gedoe voor weinig meerwaarde?
| Buttons are pills at `30px`; everything else uses the tighter `0.25rem`. The shadow is a flat | ||
| ambient glow with no y-offset, not Bootstrap's lifted drop shadow. | ||
|
|
||
| **Focus.** The bundle keeps Bootstrap 5.2's focus model rather than 5.3's `$focus-ring-*` |
Member
There was a problem hiding this comment.
Eventueel moet dit (later) ook eens herbekeken worden?
| `mb-3` / `gy-3` between them. | ||
| - Edit compiled CSS in the application's `public/assets/`. | ||
|
|
||
| ## Open questions |
Member
There was a problem hiding this comment.
Moeten al die TODO's niet eens nagekeken worden?
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
2 participants
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.
Why
The bundle ships the shared admin theme every project inherits, but nothing stated its design decisions in one place. Working that out meant reading 1763 lines of Sass variables to separate deliberate choices from stock Bootstrap. Establishing that reference turned up three real defects, fixed here alongside it.
What changed
fix:SCSS entry-point import ordererror.scssandmail.scssimported Bootstrap'svariablesbeforebootstrap-variables. Both sides use!default, so the vendor value won and every bundle override was silently inert — error pages and emails rendered with stock Bootstrap tokens.error.scssalso never importedbootstrap-variables-dark, yetbase_error.html.twigincludessettheme.html.twigand so does getdata-bs-theme. The dark custom properties were never emitted whilebase/_error.scssreadsvar(--error-bg)andvar(--error-content-bg), so dark error pages used light colours.This is the only behavioural change in the PR.
docs:DESIGN.mdNew reference at the repo root in the designmd.ai format, linked from
docs/index.md. Every value diffed against stock Bootstrap 5.3.3 so only deliberate decisions are recorded. Includes the card-based page composition rule, a 37-entry partial inventory matching_imports.scss, and 11 open questions recorded rather than fixed.docs:crud.md template inheritanceThe three page examples extended
'layout.html.twig'and filled{% block content %}. Neither exists —layout.html.twigis absent fromapplication-skeletonand every sibling project, and the bundle'sbase.html.twigdefinesmain, notcontent. Examples now also follow the card composition rule.docs:twig-cs-fixer errorsAll 16 twig snippets across
docs/now lint clean (was 9 errors in 5 files).Verification
Bootstrap is not installed in this repo, so the SCSS was compiled by symlinking a sibling project's
vendor/twbsinto a fake vendor tree. All three entry points compile clean:style.scss434KB,error.scss280KB,mail.scss34KB.Before/after on the compiled
error.css:--bs-border-radius0.375rem(stock)0.25rem--bs-gray-900#212529(stock)#2d2f35--bs-body-bg#fff(stock)#f8f9fa--error-bg#4B6376mail.csschanged by exactly 4 lines:$dark-graynow resolves to#383a43instead of stock#212529.Documentation accuracy was checked mechanically: every file path resolves, 10 spot-checked values trace to real source lines, the inventory count matches
_imports.scssexactly (37 = 37), and each twig snippet was extracted to a real.twigfile and linted.sumo hooks:checkpasses on all changed files. The bundle's own 23 templates still lint clean.Test plan
#4B6376rather than the light#b2cadd#383a43style.scssis unchanged by this PR)Error pages and emails visibly change appearance. That is the intended correction, but it is a change to live output and deserves a look in a browser and a real test email.
Notes for review
{% block main %}. The doc says to treat those as unconverted rather than counter-examples. No consuming project is changed here.$white: #e1e1e1in the dark variables file (proven dead —#e1e1e1appears zero times in 434KB of output), the undeclared--user-bgconsumed by.user-navatlayouts/_header.scss:22, andtwbs/bootstrapbeing an undeclared peer dependency.🤖 Generated with Claude Code
Summary by Sourcery
Document the admin design system and correct theme compilation, component styling, and documentation examples.
Bug Fixes:
Enhancements:
Documentation: