Skip to content

fix: icon inline size#6914

Open
nmerget wants to merge 3 commits into
mainfrom
fix-icon-inline-size
Open

fix: icon inline size#6914
nmerget wants to merge 3 commits into
mainfrom
fix-icon-inline-size

Conversation

@nmerget
Copy link
Copy Markdown
Collaborator

@nmerget nmerget commented Jun 5, 2026

Proposed changes

If an icon-font is not loading, the icon name will destroy a layout causing layout-shifts

image

Checklist

Code Quality

  • I have reviewed my own code (self-review), including the screen- and snapshots
  • I have reviewed my changes locally with an AI assistant (e.g. GitHub Copilot, Kiro, etc.)
  • No hardcoded values, magic numbers, or debug code left in

Validation

  • I have added/updated tests that cover my changes
  • I have tested across all relevant frameworks (React, Angular, Vue) if applicable

General

  • The PR title follows the conventional commits format (e.g. feat:, fix:, chore:), as in Git Commit Conventions
  • If architecture, structure, or conventions changed inside a packages/* folder, the corresponding AGENTS.md has been updated

🔭🐙🐈 Test this branch here: https://design-system.deutschebahn.com/core-web/review/fix-icon-inline-size

nmerget added 2 commits June 5, 2026 15:20
aspect-ratio: 1 alone does not work reliably in all cases, so inline-size is needed alongside it.
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Jun 5, 2026

🦋 Changeset detected

Latest commit: 73054f7

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 13 packages
Name Type
@db-ux/core-foundations Patch
@db-ux/core-components Patch
@db-ux/ngx-core-components Patch
@db-ux/react-core-components Patch
@db-ux/wc-core-components Patch
@db-ux/v-core-components Patch
@db-ux/core-stylelint Patch
@db-ux/core-migration Patch
@db-ux/agent-cli Patch
@db-ux/core-eslint-plugin Patch
@db-ux/core-vite-plugin Patch
@db-ux/core-postcss-plugin Patch
@db-ux/mcp-server Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions Bot added 📕documentation Improvements or additions to documentation 🏗foundations labels Jun 5, 2026
@nmerget nmerget moved this from 🏗 In progress to 🎁 Ready for review in UX Engineering Team Backlog Jun 5, 2026
@nmerget nmerget enabled auto-merge (squash) June 5, 2026 13:26
@@ -162,6 +162,7 @@ $default-icon-font-size: var(--db-icon-font-size, #{$default-icon-size-rem});
vertical-align: var(--db-icon-vertical-align, middle);
/* stylelint-disable-next-line db-ux/use-sizing */
block-size: $default-icon-font-size;
Copy link
Copy Markdown
Collaborator

@mfranzke mfranzke Jun 5, 2026

Choose a reason for hiding this comment

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

Suggested change
block-size: $default-icon-font-size;

@nmerget as I don't know how to reproduce those "all cases", could we get rid of (or in other words, replace inline-size by) block-size, as we still have aspect-ratio, so does that aspect-ratio work in that direction ? to me it looks like that we could get rid of block-size than.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The icon has to be square. Which is the normal case, but when the font is not loaded the :before just shows the content which is a text. For the text aspect-ratio is not applied correctly, inline-size does.

There is only one line extra but it makes sure that the layout won't break

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

but than we could remove aspect-ratio.

"@db-ux/core-foundations": patch
---

fix: Re-add `inline-size` to icon styles because `aspect-ratio: 1` alone does not work reliably in all cases.
Copy link
Copy Markdown
Collaborator

@mfranzke mfranzke Jun 5, 2026

Choose a reason for hiding this comment

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

Suggested change
fix: Re-add `inline-size` to icon styles because `aspect-ratio: 1` alone does not work reliably in all cases.
fix: ensure that the icon fallback is displayed as a rectangle

that was what this PR does "internally", but we should tell our users why we do it or in other words what it does fix for them.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I didn't replace it. I add inline-size to have both

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

that's fine, but still the message needs to tell the users what is changing to them, not to us or our code.

@mfranzke mfranzke moved this from 🎁 Ready for review to 🎶 Waiting for feedback in UX Engineering Team Backlog Jun 5, 2026
@mfranzke mfranzke requested a review from Copilot June 5, 2026 13:50
@mfranzke mfranzke changed the title Fix icon inline size fix: icon inline size Jun 5, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses layout shifts that occur when an icon font fails to load by explicitly constraining the icon placeholder’s inline dimension, ensuring the icon name fallback cannot expand the layout unexpectedly.

Changes:

  • Add inline-size to the shared %icon styles (alongside existing block-size) to enforce consistent icon dimensions.
  • Add a Changeset to publish the fix as a patch for @db-ux/core-foundations.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
packages/foundations/scss/icons/_icon-helpers.scss Adds inline-size to the %icon placeholder to prevent layout shifts when icon-font fallback text appears.
.changeset/fix-icon-inline-size.md Declares a patch release for @db-ux/core-foundations describing the sizing fix.

Comment thread packages/foundations/scss/icons/_icon-helpers.scss
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@nmerget nmerget moved this from 🎶 Waiting for feedback to 🎁 Ready for review in UX Engineering Team Backlog Jun 5, 2026
@nmerget nmerget requested a review from mfranzke June 5, 2026 14:21
@mfranzke mfranzke moved this from 🎁 Ready for review to 👀 Actively In Review in UX Engineering Team Backlog Jun 5, 2026
@mfranzke mfranzke moved this from 👀 Actively In Review to 🎶 Waiting for feedback in UX Engineering Team Backlog Jun 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

📕documentation Improvements or additions to documentation 🏗foundations

Projects

Status: 🎶 Waiting for feedback

Development

Successfully merging this pull request may close these issues.

3 participants