WB-1617: Remove Icon component #2128

jandrade · 2023-11-27T19:32:42Z

Summary:

This removes the Icon component from Wonder Blocks. This is possible now that we
have fully migrated to the new icon system! (Phosphor).

Issue: WB-1617

Test plan:

Verify that the tests pass and the docs look good.

changeset-bot · 2023-11-27T20:18:03Z

🦋 Changeset detected

Latest commit: f19069e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 15 packages

Name	Type
@khanacademy/wonder-blocks-icon	Major
@khanacademy/wonder-blocks-accordion	Patch
@khanacademy/wonder-blocks-banner	Patch
@khanacademy/wonder-blocks-birthday-picker	Patch
@khanacademy/wonder-blocks-button	Patch
@khanacademy/wonder-blocks-dropdown	Patch
@khanacademy/wonder-blocks-form	Patch
@khanacademy/wonder-blocks-icon-button	Patch
@khanacademy/wonder-blocks-link	Patch
@khanacademy/wonder-blocks-search-field	Patch
@khanacademy/wonder-blocks-switch	Patch
@khanacademy/wonder-blocks-modal	Patch
@khanacademy/wonder-blocks-popover	Patch
@khanacademy/wonder-blocks-pill	Patch
@khanacademy/wonder-blocks-tooltip	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

khan-actions-bot · 2023-11-27T20:20:25Z

Gerald

Required Reviewers

@Khan/wonder-blocks for changes to .changeset/itchy-books-laugh.md, consistency-tests/__tests__/clickables.test.tsx, consistency-tests/__tests__/ref-forwarded.test.tsx, packages/wonder-blocks-icon/src/index.ts, packages/wonder-blocks-icon/src/types.ts, packages/wonder-blocks-icon/src/components/phosphor-icon.tsx, packages/wonder-blocks-icon/src/util/icon-util.test.ts, packages/wonder-blocks-icon/src/util/icon-util.ts

Don't want to be involved in this pull request? Comment #removeme and we won't notify you of further changes.

github-actions · 2023-11-27T20:21:54Z

Size Change: -2.1 kB (-2%)

Total Size: 90.8 kB

Filename	Size	Change
`packages/wonder-blocks-icon/dist/es/index.js`	1.06 kB	-2.1 kB (-66%)	🏆

ℹ️ View Unchanged

Filename	Size
`packages/wonder-blocks-accordion/dist/es/index.js`	3.67 kB
`packages/wonder-blocks-banner/dist/es/index.js`	1.53 kB
`packages/wonder-blocks-birthday-picker/dist/es/index.js`	1.72 kB
`packages/wonder-blocks-breadcrumbs/dist/es/index.js`	1.13 kB
`packages/wonder-blocks-button/dist/es/index.js`	4.27 kB
`packages/wonder-blocks-cell/dist/es/index.js`	2.19 kB
`packages/wonder-blocks-clickable/dist/es/index.js`	3.24 kB
`packages/wonder-blocks-color/dist/es/index.js`	1.15 kB
`packages/wonder-blocks-core/dist/es/index.js`	3.67 kB
`packages/wonder-blocks-data/dist/es/index.js`	6.33 kB
`packages/wonder-blocks-dropdown/dist/es/index.js`	12 kB
`packages/wonder-blocks-form/dist/es/index.js`	5.34 kB
`packages/wonder-blocks-grid/dist/es/index.js`	1.36 kB
`packages/wonder-blocks-i18n/dist/es/index.js`	4.54 kB
`packages/wonder-blocks-icon-button/dist/es/index.js`	3.06 kB
`packages/wonder-blocks-labeled-field/dist/es/index.js`	72 B
`packages/wonder-blocks-layout/dist/es/index.js`	1.88 kB
`packages/wonder-blocks-link/dist/es/index.js`	2.54 kB
`packages/wonder-blocks-modal/dist/es/index.js`	5.04 kB
`packages/wonder-blocks-pill/dist/es/index.js`	1.03 kB
`packages/wonder-blocks-popover/dist/es/index.js`	4.33 kB
`packages/wonder-blocks-progress-spinner/dist/es/index.js`	1.51 kB
`packages/wonder-blocks-search-field/dist/es/index.js`	1.55 kB
`packages/wonder-blocks-spacing/dist/es/index.js`	158 B
`packages/wonder-blocks-switch/dist/es/index.js`	2.06 kB
`packages/wonder-blocks-testing/dist/es/index.js`	3.94 kB
`packages/wonder-blocks-theming/dist/es/index.js`	1.21 kB
`packages/wonder-blocks-timing/dist/es/index.js`	1.78 kB
`packages/wonder-blocks-toolbar/dist/es/index.js`	862 B
`packages/wonder-blocks-tooltip/dist/es/index.js`	5.05 kB
`packages/wonder-blocks-typography/dist/es/index.js`	1.49 kB

_{compressed-size-action}

github-actions · 2023-11-27T20:22:16Z

npm Snapshot: Published

🎉 Good news!! We've packaged up the latest commit from this PR (14d4d83) and published all packages with changesets to npm.

You can install the packages in webapp by running:

./services/static/dev/tools/deploy_wonder_blocks.js --tag="PR2128"

Packages can also be installed manually by running:

yarn add @khanacademy/wonder-blocks-<package-name>@PR2128

codecov · 2023-11-27T20:22:28Z

Codecov Report

Merging #2128 (f19069e) into main (393ea54) will decrease coverage by 0.02%.
The diff coverage is 100.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2128      +/-   ##
==========================================
- Coverage   97.06%   97.04%   -0.02%     
==========================================
  Files         243      241       -2     
  Lines       28233    27989     -244     
  Branches     2469     2377      -92     
==========================================
- Hits        27404    27163     -241     
+ Misses        829      826       -3

Files	Coverage Δ
...onder-blocks-icon/src/components/phosphor-icon.tsx	`98.65% <100.00%> (+0.03%)`	⬆️
packages/wonder-blocks-icon/src/util/icon-util.ts	`100.00% <100.00%> (+4.91%)`	⬆️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 393ea54...f19069e. Read the comment docs.

github-actions · 2023-11-27T20:23:52Z

A new build was pushed to Chromatic! 🚀

https://5e1bf4b385e3fb0020b7073c-uuksgpxsyu.chromatic.com/

Chromatic results:

Metric	Total
Captured snapshots	315
Tests with visual changes	2
Total stories	387
Inherited (not captured) snapshots [TurboSnap]	0
Tests on the build	315

somewhatabstract

Awesome! Congrats!

My review uses Conventional Comments to add context and set expectations for the comments I am leaving.

somewhatabstract · 2023-11-27T20:40:37Z

.changeset/itchy-books-laugh.md

+"@khanacademy/wonder-blocks-icon": major
+---
+
+Delete Icon component in favor of PhosphorIcon


praise: ❗ 😮 ❗

somewhatabstract · 2023-11-27T20:42:23Z

packages/wonder-blocks-icon/src/types.ts

+/**
+ * All the possible icon sizes.
+ */
+export type IconSize = "small" | "medium" | "large" | "xlarge";


question: Since in TypeScript 5, all enums are union enums, I wonder if we should do an enum here instead of a string union? 🤷🏻 I don't feel strongly about it. Just asking the question.

That's fair, I'll make the change. Thanks for the suggestion!

After quickly making the change, I realized that this would mean that we'd have to replace a bunch of callers to make it compatible with this change. (same would happen with Button, IconButton, etc).

I've filed a ticket to evaluate this later: https://khanacademy.atlassian.net/browse/WB-1639

Remove Icon component

07a06be

jandrade self-assigned this Nov 27, 2023

khan-actions-bot requested a review from a team November 27, 2023 20:20

somewhatabstract approved these changes Nov 27, 2023

View reviewed changes

jandrade added 2 commits November 28, 2023 10:27

Address feedback (enum)

dd1b2db

Revert enum change

f19069e

jandrade merged commit 171e3b0 into main Nov 28, 2023
13 checks passed

jandrade deleted the phosphor-rm-icon branch November 28, 2023 15:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WB-1617: Remove Icon component #2128

WB-1617: Remove Icon component #2128

jandrade commented Nov 27, 2023

changeset-bot bot commented Nov 27, 2023 •

edited

Loading

khan-actions-bot commented Nov 27, 2023 •

edited

Loading

github-actions bot commented Nov 27, 2023 •

edited

Loading

github-actions bot commented Nov 27, 2023 •

edited

Loading

codecov bot commented Nov 27, 2023 •

edited

Loading

github-actions bot commented Nov 27, 2023 •

edited

Loading

somewhatabstract left a comment •

edited

Loading

somewhatabstract Nov 27, 2023

somewhatabstract Nov 27, 2023

jandrade Nov 28, 2023

jandrade Nov 28, 2023 •

edited

Loading

WB-1617: Remove Icon component #2128

WB-1617: Remove Icon component #2128

Conversation

jandrade commented Nov 27, 2023

Summary:

Test plan:

changeset-bot bot commented Nov 27, 2023 • edited Loading

🦋 Changeset detected

khan-actions-bot commented Nov 27, 2023 • edited Loading

Gerald

github-actions bot commented Nov 27, 2023 • edited Loading

github-actions bot commented Nov 27, 2023 • edited Loading

npm Snapshot: Published

codecov bot commented Nov 27, 2023 • edited Loading

Codecov Report

github-actions bot commented Nov 27, 2023 • edited Loading

A new build was pushed to Chromatic! 🚀

Chromatic results:

somewhatabstract left a comment • edited Loading

Choose a reason for hiding this comment

somewhatabstract Nov 27, 2023

Choose a reason for hiding this comment

somewhatabstract Nov 27, 2023

Choose a reason for hiding this comment

jandrade Nov 28, 2023

Choose a reason for hiding this comment

jandrade Nov 28, 2023 • edited Loading

Choose a reason for hiding this comment

changeset-bot bot commented Nov 27, 2023 •

edited

Loading

khan-actions-bot commented Nov 27, 2023 •

edited

Loading

github-actions bot commented Nov 27, 2023 •

edited

Loading

github-actions bot commented Nov 27, 2023 •

edited

Loading

codecov bot commented Nov 27, 2023 •

edited

Loading

github-actions bot commented Nov 27, 2023 •

edited

Loading

somewhatabstract left a comment •

edited

Loading

jandrade Nov 28, 2023 •

edited

Loading