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(icon): icon component migration #537

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

feat(icon): icon component migration #537

wants to merge 13 commits into from

Conversation

ashley-hunter
Copy link
Collaborator

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

Which package are you modifying?

  • accordion
  • alert
  • alert-dialog
  • aspect-ratio
  • avatar
  • badge
  • button
  • calendar
  • card
  • checkbox
  • collapsible
  • combobox
  • command
  • context-menu
  • data-table
  • date-picker
  • dialog
  • dropdown-menu
  • hover-card
  • icon
  • input
  • label
  • menubar
  • navigation-menu
  • pagination
  • popover
  • progress
  • radio-group
  • scroll-area
  • select
  • separator
  • sheet
  • skeleton
  • slider
  • sonner
  • spinner
  • switch
  • table
  • tabs
  • textarea
  • toast
  • toggle
  • tooltip
  • typography

What is the current behavior?

Currently we wrap the NgIcon component which adds a hard dependency on the library and more specifically can lead to us being tied to a specific version and make it hard to update if there are ever any breaking changes.

This pull request changes the component we had to a directive that applies the little bit of custom styling we add, this removes the hard dependency on any specific version of NgIcons.

What is the new behavior?

We use the ng-icon component directly and add an attribute for the styling

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

I have added an automated migration to assist in the process of migration as this is a breaking change

@ashley-hunter ashley-hunter marked this pull request as ready for review December 21, 2024 22:13
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.

1 participant