Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: Use inline SVGs for Radio icon #1460

Merged
merged 35 commits into from
Sep 25, 2024

Conversation

alimpens
Copy link
Contributor

@alimpens alimpens commented Jul 26, 2024

Describe the pull request

Thank you for contributing to the project!
Please use this template to help us handle your PR smoothly.

What

It uses an inline SVG for the Radio icon and allows users to replace this icon. It also styles the default icon for correct display in forced colors mode.

Why

This makes it easier for external users to change the Icon. They can use their own SVGs and style them however they want to. This PR also fixes the display of Radios in forced colors mode.

How

By using an inline SVG with ams-radio__circle and ams-radio__checked-indicator classes, and styling those.

Checklist

Before submitting your pull request, please ensure you have done the following. Check each checkmark if you have done so or if it wasn't necessary:

  • Add or update unit tests
  • Add or update documentation
  • Add or update stories
  • Add or update exports in index.* files
  • Start the PR title with a Conventional Commit prefix, as explained here.

@github-actions github-actions bot temporarily deployed to demo-DES-649-use-svg-for-radio July 26, 2024 15:21 Destroyed
@alimpens alimpens changed the title feat!: Use svg for Radio feat!: Use inline SVGs for Radio Jul 26, 2024
@alimpens alimpens changed the title feat!: Use inline SVGs for Radio feat!: Use inline SVGs for Radio icon Jul 26, 2024
@alimpens alimpens marked this pull request as draft July 27, 2024 07:29
@github-actions github-actions bot temporarily deployed to demo-DES-649-use-svg-for-radio August 12, 2024 14:48 Destroyed
@alimpens alimpens marked this pull request as ready for review August 12, 2024 14:53
@github-actions github-actions bot temporarily deployed to demo-DES-649-use-svg-for-radio August 12, 2024 14:54 Destroyed
Copy link
Contributor

@dlnr dlnr left a comment

Choose a reason for hiding this comment

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

Looks like an improvement, some non token values that look weird to me.

packages/css/src/components/radio/radio.scss Outdated Show resolved Hide resolved
packages/css/src/components/radio/radio.scss Outdated Show resolved Hide resolved
@dlnr
Copy link
Contributor

dlnr commented Aug 13, 2024

This alignment of the text seems slightly off:

image

Figma

image

Storybook

@github-actions github-actions bot temporarily deployed to demo-DES-649-use-svg-for-radio September 20, 2024 08:14 Destroyed
@github-actions github-actions bot temporarily deployed to demo-DES-649-use-svg-for-radio September 20, 2024 09:00 Destroyed
@github-actions github-actions bot temporarily deployed to demo-DES-649-use-svg-for-radio September 20, 2024 12:38 Destroyed
@github-actions github-actions bot temporarily deployed to demo-DES-649-use-svg-for-radio September 20, 2024 12:42 Destroyed
@github-actions github-actions bot temporarily deployed to demo-DES-649-use-svg-for-radio September 20, 2024 13:13 Destroyed
@github-actions github-actions bot temporarily deployed to demo-DES-649-use-svg-for-radio September 20, 2024 13:35 Destroyed
@github-actions github-actions bot temporarily deployed to demo-DES-649-use-svg-for-radio September 25, 2024 13:32 Destroyed
@VincentSmedinga VincentSmedinga merged commit c19055b into develop Sep 25, 2024
6 checks passed
@VincentSmedinga VincentSmedinga deleted the feat/DES-649-use-svg-for-radio branch September 25, 2024 14:10
This was referenced Oct 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants