Skip to content

chore: Replaced hardcoded css values with design tokens - #257

Merged
georgylobko merged 2 commits into
mainfrom
chore/replace-hardcoded-values-with-token
Sep 4, 2026
Merged

chore: Replaced hardcoded css values with design tokens#257
georgylobko merged 2 commits into
mainfrom
chore/replace-hardcoded-values-with-token

Conversation

@georgylobko

Copy link
Copy Markdown
Member

Description

Replaced hardcoded border radiuses with Cloudscape design tokens, because they cause a regression in the 1T. No visual changes for VR.

Related links, issue #, if available: n/a

How has this been tested?

Review checklist

The following items are to be evaluated by the author(s) and the reviewer(s).

Correctness

  • Changes include appropriate documentation updates.
  • Changes are backward-compatible if not indicated, see CONTRIBUTING.md.
  • Changes do not include unsupported browser features, see CONTRIBUTING.md.
  • Changes were manually tested for accessibility, see accessibility guidelines.

Security

Testing

  • Changes are covered with new/existing unit tests?
  • Changes are covered with new/existing integration tests?

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Comment thread src/core/styles.scss Outdated
padding-inline: cs.$space-static-m;
background-color: cs.$color-background-container-content;
border-radius: 8px;
border-radius: cs.$border-radius-popover;

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I also considered introducing a new design token for it, but using the existing popover token seems appropriate here

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think a specific design token is a better option. Although the popover border radius happens to have the same default value as this element, they are semantically distinct. Would everyone who customizes the popover border radius expect the chart empty state indicator to change as well? I'd say not necessarily.

Alternatively, maybe border-radius-card-embedded is acceptable, since this is similar to a card embedded in a chart.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

$border-radius-card-embedded sounds like a good option, addressed. I wouldn’t introduce a new token for this element. Although the elements are semantically different, border radiuses should remain consistent across similar components in the design system

@georgylobko
georgylobko marked this pull request as ready for review September 3, 2026 14:29
@georgylobko
georgylobko requested a review from a team as a code owner September 3, 2026 14:29
@georgylobko
georgylobko requested review from jperals and removed request for a team September 3, 2026 14:29
@georgylobko
georgylobko added this pull request to the merge queue Sep 4, 2026
Merged via the queue into main with commit 1ba66df Sep 4, 2026
43 of 44 checks passed
@georgylobko
georgylobko deleted the chore/replace-hardcoded-values-with-token branch September 4, 2026 09:05
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