Skip to content

Try zoom instead of css transform for rendering xsmll icons on low de…#380

Draft
ChristianAlbrecht wants to merge 1 commit intomasterfrom
small-icon-tests
Draft

Try zoom instead of css transform for rendering xsmll icons on low de…#380
ChristianAlbrecht wants to merge 1 commit intomasterfrom
small-icon-tests

Conversation

@ChristianAlbrecht
Copy link
Copy Markdown
Contributor

@ChristianAlbrecht ChristianAlbrecht commented Mar 26, 2026

Do not merge yet! Needs some testing/verification yet if the proposed changes actually fix something or not.

Copilot AI review requested due to automatic review settings March 26, 2026 14:49
@ChristianAlbrecht ChristianAlbrecht requested a review from a team as a code owner March 26, 2026 14:49
@ChristianAlbrecht ChristianAlbrecht requested review from knime-ghub-bot and removed request for a team March 26, 2026 14:49
@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Mar 26, 2026

⚠️ No Changeset found

Latest commit: 548cd4a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

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 adjusts how xsmall icons are rendered on low-density displays, aiming to reduce subpixel-related SVG rendering artifacts by changing the scaling approach in the icon CSS.

Changes:

  • Tightened the low-density screen media query threshold from 1.5dppx to 1.25dppx.
  • Replaced transform: scale(...) with zoom: ... for scaling xsmall icons and added shape-rendering: crispEdges in that branch.
Comments suppressed due to low confidence (1)

packages/components/src/accessories/Icon/styles.css:25

  • The explanatory comment still says the solution is to apply a “CSS transform”, but the implementation now uses zoom (and shape-rendering: crispEdges). Update the comment so it accurately reflects the current approach and rationale.
      /* For small icons on low density screens, we can't simply set the sizes as this leads to subpixel
      rendering issues which manifests in misaligned or 'jumping' svg content.
      Solution is to render the SVG in it's natural size (don't indirectly modify the viewBox) and instead apply a
      CSS transform. Thus the position of the SVG elements stays intact. */

Comment on lines 31 to +34
var(--kds-border-width-icon-stroke-s) / var(--scaling-factor)
);
transform: scale(var(--scaling-factor));
zoom: var(--scaling-factor);
shape-rendering: crispEdges;
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

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

zoom is a non-standard CSS property and is not supported in Firefox. In browsers that ignore it, this media-query branch will still apply --icon-width/height: 0.75x (and the adjusted stroke width) but won’t scale the element down, so xsmall icons will render at the wrong size. Consider adding a @supports (zoom: 1) guard with a transform: scale(...) (or other) fallback for browsers without zoom, or restructure so the fallback keeps the intended visual and layout size.

Copilot uses AI. Check for mistakes.
@sonarqubecloud
Copy link
Copy Markdown

);
transform: scale(var(--scaling-factor));
zoom: var(--scaling-factor);
shape-rendering: crispEdges;
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.

cripsEdges works only for rectangular icons

--icon-stroke-width: var(--kds-border-width-icon-stroke-s);

@media (max-resolution: 1.5dppx) {
@media (max-resolution: 1.25dppx) {
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.

Does not seem to change anything for me

@RainerSchmoeger RainerSchmoeger marked this pull request as draft March 26, 2026 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants