Add a rule that warn against removing unsafeDisableTooltip prop from IconButton

[broccolinisoup]

Based on the icon button roll out plan (reference - internal only), I pushed a rule to motivate adoption of tooltips in icon buttons.

The rule has an auto fix which simply removes the unsafeDisableTooltip prop. The plan is to include this rule to score cards (still in the process of clarifying this with @TylerJDev and @khiga8 ) and enable it at dotcom along with disabling the rule for each instances of icon buttons.

Current

<IconButton aria-label="Search" icon={SearchIcon} onClick={onClick} />

Aim

// eslint-disable-next-line a11y-remove-disable-tooltip
<IconButton aria-label="Search" icon={SearchIcon} onClick={onClick} unsafeDisableTooltip={true} />

Question

One of the steps of the roll out is to provide resources for product teams to give them the adoption plan that includes testing the icon button after removing unsafeDisableTooltip prop. I wonder if we can achieve this via this rule? Maybe including this on the docs and providing a url of the rule? I am open to any ideas for sharing the path with the teams.

[changeset-bot]

🦋 Changeset detected

Latest commit: 9f538d4

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

This PR includes changesets to release 1 package

Name	Type
eslint-plugin-primer-react	Minor

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

[siddharthkp]

LGTM!

[siddharthkp]

(After we change the default value of unsafeDisableTooltip), Do you have a preference for unsafeDisableTooltip={false} vs not having it all? Should we add that to the linter?

[TylerJDev]

To add to this, is the plan to deprecate the prop in the future once we know unsafeDisableTooltip={true} isn't being used anywhere in dotcom (or at least a very low amount of usage)?

[broccolinisoup]

Do you have a preference for unsafeDisableTooltip={false} vs not having it all? Should we add that to the linter?

I really like the idea of adding it to the linter! I always imagined that we would remove the prop instead of unsafeDisableTooltip={false} - I'll add it to the rule.

is the plan to deprecate the prop in the future once we know unsafeDisableTooltip={true} isn't being used anywhere in dotcom (or at least a very low amount of usage)?

This is the hope and the plan 🙌🏻 - however there might be some rare cases that we need to hide the tooltip and I guess we are yet to discover these cases as product teams test and remove unsafeDisableTooltip={true}. If we end up having icon buttons that we certainly don't need tooltips for whatever reason, we might choose to keep the prop.

[broccolinisoup]

I'll add it to the rule.

Actually, I just realised that the rule checks unsafeDisableTooltip={false} as well. Or in better words, it doesn't check the value of the prop 😆 I think this is fine because type safety will handle the value and we just need to worry about if the prop exists or not. I added a test case and into the docs though. Let me know if you have any concern 🙌🏻

[TylerJDev]

Looks good!