Skip to content

feat(ui-icons): add compare icon, include ai-colored icon in the index, improve svg to jsx conversion#2005

Merged
balzss merged 1 commit into
masterfrom
feat/compare-icon
Jun 13, 2025
Merged

feat(ui-icons): add compare icon, include ai-colored icon in the index, improve svg to jsx conversion#2005
balzss merged 1 commit into
masterfrom
feat/compare-icon

Conversation

@balzss

@balzss balzss commented Jun 3, 2025

Copy link
Copy Markdown
Contributor

INSTUI-4521
INSTUI-4563

this pr:

  • adds the new compare icon
  • replaces the 3rd party lib that was used to convert svgs to jsx with a local helper function
  • removes the temporary workaround for the ai-colored icon
  • updates the adding icons docs

test plan:

  • check the new compare and ai-colored icon in the icon list (react, svg, and icon font format)
  • check if the ai-colored examples still work in the button and heading docs
  • read the new adding icons docs

@balzss balzss self-assigned this Jun 3, 2025
@github-actions

github-actions Bot commented Jun 3, 2025

Copy link
Copy Markdown
PR Preview Action v1.6.1
Preview removed because the pull request was closed.
2025-06-13 10:03 UTC

@balzss balzss force-pushed the feat/compare-icon branch from 4e5dd4a to bb35dbe Compare June 4, 2025 13:46
@balzss balzss changed the title feat(ui-icons): wip: compare icon feat(ui-icons): add compare icon, include ai-colored icon in the index, improve svg to jsx conversion Jun 4, 2025
@balzss balzss requested review from HerrTopi and matyasf June 4, 2025 13:58
@balzss balzss marked this pull request as ready for review June 4, 2025 13:58
@balzss balzss requested review from joyenjoyer and removed request for HerrTopi June 5, 2025 09:27

@joyenjoyer joyenjoyer Jun 5, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

svg-to-jsx package should be removed from package.json if we don't use it anymore.

@balzss balzss force-pushed the feat/compare-icon branch from bb35dbe to d4d7f2c Compare June 5, 2025 10:56

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please write a test for this function.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I agree we should test this, I've opened a ticket to add tests for the ui-scripts package

@balzss balzss requested a review from joyenjoyer June 5, 2025 11:29
Comment thread docs/contributor-docs/adding-icons.md Outdated
Comment on lines 91 to 95

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.

This was a limitation with the old solution too, right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

probably yes, but it is definitely a "limitation" after this change

@balzss balzss requested review from HerrTopi and removed request for joyenjoyer June 12, 2025 09:43

@HerrTopi HerrTopi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

As we discussed offline, the guide docs should be rewritten since it contains inaccurate and hard to understand information.
This will be done in: INSTUI-4593

Nice work nontheless!

@balzss balzss force-pushed the feat/compare-icon branch from d4d7f2c to 162615f Compare June 13, 2025 09:53
@balzss balzss merged commit 13b6a04 into master Jun 13, 2025
11 checks passed
@balzss balzss deleted the feat/compare-icon branch June 13, 2025 10:03
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.

4 participants