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

allow setting icon size and fill from outside #1363

Merged
merged 8 commits into from
Jun 26, 2023
Merged

Conversation

mayank99
Copy link
Contributor

@mayank99 mayank99 commented Jun 15, 2023

Changes

Restructured the icon css slightly, adding new public-facing --iui-svg-size and --iui-svg-fill vars. These new vars will be preferred over the internal versions (prefixed with _), allowing easier modifications. The benefit of this would be that we wouldn't need to duplicate icon styling in a bunch of components, as we could simply add iui-svg-icon in the html and then specify these css vars inside the component css.

Testing

All* existing usage is unchanged. Setting size and fill from outside is reflected correctly.

*had to change expandable-block.html to add missing class.

Docs

added changeset.

@mayank99 mayank99 added this to the React 3.0 milestone Jun 15, 2023
@mayank99 mayank99 self-assigned this Jun 15, 2023
@mayank99 mayank99 marked this pull request as ready for review June 16, 2023 15:39
@mayank99 mayank99 requested review from a team as code owners June 16, 2023 15:39
@mayank99 mayank99 requested review from gretanausedaite and LostABike and removed request for a team June 16, 2023 15:39
Copy link
Contributor

@LostABike LostABike left a comment

Choose a reason for hiding this comment

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

Everything icons seem to be working correctly/as before 👍

@mayank99 mayank99 merged commit 92f81ad into dev Jun 26, 2023
@mayank99 mayank99 deleted the mayank/icon-rework branch June 26, 2023 15:46
@imodeljs-admin imodeljs-admin mentioned this pull request Oct 23, 2023
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