Icon: Support size prop for dashicons

[mirka]

What problem does this address?

The size prop does not work when the icon is a dashicon string.

What is your proposed solution?

Add some handling (either to the Dashicon or the Icon component) so it adds a font-size style equivalent to the size.

[t-hamano]

In the Icon component, if the size property is to be applied to the dashicon, does this mean that the default font size will be 24px instead of the current 20px?

Personally, I would expect the current default size (font size) to remain 20px when size prop is not specified in the dashicon. Developers still using the dashicon should expect the font size to remain 20px unless they explicitly specify otherwise.
If we plan to deprecate the dashicon as you mention in #43725, how about keeping the default size?

I believe the direction of this issue will help move #43574 forward 🙂
What do you think?

[mirka]

If we plan to deprecate the dashicon as you mention in #43725, how about keeping the default size?

Yes, if it's agreed that dashicons can be deprecated, there's probably no good reason to add a "new feature" (even if it's kind of a bug fix) like customizable sizes. So if we're going to deprecate, I think keeping the current 20px size is reasonable 👍

I just wasn't sure if there would be pushback on deprecating dashicons, since I hear it's widely used by the community 😅 At least, the contributors to this repo generally seem to agree that it can/should be deprecated.

(cc @ciampo for visibility)

[ciampo]

My preferred option would be to deprecate dashicons (ie. to simplify rather than add more complexity), but we should definitely check with the project leads if this something that we can do.

[t-hamano]

I would like to address this problem, but first I think it would be a good idea to rewrite the test from Enzyme to React Testing Library to clarify the current specifications.
Does this make sense?

[ciampo]

rewrite the test from Enzyme to React Testing Library to clarify the current specifications.

That would always be a good idea, as we're in the process of migrating from Enzyme to RTL for the whole repo :)

[t-hamano]

Thank you! I'll work on refactoring the test first.

[t-hamano]

I have submitted a PR regarding the refactoring of tests: #44051