Conversation
Removed custom font family properties for sans and head, as those are already forwarded from `packages/foundations/scss/defaults/_default-properties.scss`
|
We might provide this as a seperate whitelabel default-theme CSS, but remove it in total as a simplification for current testing.
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR refactors the CSS Custom Properties setup in the DB UX Design System's core foundations package to reduce the number of CSS Custom Variables and @property declarations in bundled outputs. It removes the whitelabel default theme from bundled CSS entry points and cleans up redundant font-family property declarations.
Changes:
- Removed
@forward "defaults/default-theme"fromrelative.scss, so bundled CSS files (relative.css,absolute.css,rollup.css,webpack.css) no longer include the default theme (it remains available as a standalonedefault-theme.cssimport) - Removed duplicate
--db-font-family-sansand--db-font-family-head@propertydeclarations fromdefault-theme.scss(already defined in the forwarded_default-properties.scss) - Added CSS Custom Properties for container sizes and screen breakpoints as a delta between Core Foundations and DB Theme packages in
default-theme.scss
Reviewed changes
Copilot reviewed 2 out of 6 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
packages/foundations/scss/relative.scss |
Removed the forward of default-theme from bundled CSS entry point |
packages/foundations/scss/defaults/default-theme.scss |
Removed redundant font-family properties; added container and screen size @property declarations |
__snapshots__/tooltip/showcase/chromium-highContrast/... |
Updated visual regression screenshot reflecting CSS changes |
| @property --db-screen-md { | ||
| syntax: "*"; | ||
| inherits: true; | ||
| initial-value: 64em; | ||
| } | ||
|
|
||
| @property --db-screen-sm { | ||
| syntax: "*"; | ||
| inherits: true; | ||
| initial-value: 48em; | ||
| } | ||
|
|
||
| @property --db-screen-xl { | ||
| syntax: "*"; | ||
| inherits: true; | ||
| initial-value: 120em; | ||
| } | ||
|
|
||
| @property --db-screen-xs { | ||
| syntax: "*"; | ||
| inherits: true; | ||
| initial-value: 20em; | ||
| } |
There was a problem hiding this comment.
The new --db-screen-* @property declarations are missing --db-screen-lg. The SCSS variable $db-screen-lg: var(--db-screen-lg) is defined in _variables.scss (line 97), and it's also referenced in tailwind/theme/_variables.scss (line 32), but no @property --db-screen-lg is declared here or anywhere else. Since you're adding screen property declarations in this file, --db-screen-lg should be included as well (with initial-value: 90em, i.e. px-to-em(1440)).
Comment out the forwarding of default-properties in SCSS.
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This reverts commit b6c269b.
those are already forwarded via default-container-properties.scss
|
closed in favour of #6308 |
Proposed changes
We need to identify ways of how to reduce the amount of CSS Custom Variables and related
@propertydeclarations, only to the ones that are necessarily to get included.packages/foundations/scss/defaults/_default-properties.scssdefault-themefrom CSS, as we might want to provide this as a separate file, but it's most likely unnecessary in our DB context (and even for others as most of them would be using a theme).packages/foundations/scss/defaults/default-theme.scssTypes of changes
Further comments
🔭🐙🐈 Test this branch here: https://design-system.deutschebahn.com/core-web/review/refactor-clean-up-css-custom-properties