rusackas commented on code in PR #36127:
URL: https://github.com/apache/superset/pull/36127#discussion_r2621568139
##########
superset-frontend/src/features/alerts/components/RecipientIcon.tsx:
##########
@@ -52,6 +52,12 @@ export default function RecipientIcon({ type }: { type:
string }) {
);
recipientIconConfig.label = NotificationMethodOption.Slack;
break;
+ case NotificationMethodOption.Webhook:
+ recipientIconConfig.icon = (
+ <Icons.ApiOutlined css={notificationStyledIcon} iconSize="l" />
Review Comment:
Valid concern, but pre-existing pattern
Looking at the code:
tsxcase NotificationMethodOption.Webhook:
recipientIconConfig.icon = (
<Icons.ApiOutlined css={notificationStyledIcon} iconSize="l" />
);
recipientIconConfig.label = NotificationMethodOption.Webhook;
break;
This follows the exact same pattern as the existing Email and Slack cases:
tsxcase NotificationMethodOption.Email:
recipientIconConfig.icon = (
<Icons.MailOutlined css={notificationStyledIcon} iconSize="l" />
);
recipientIconConfig.label = NotificationMethodOption.Email;
break;
The label is set and presumably used by the parent component (likely as a
tooltip or for screen reader text). Whether that's sufficient for accessibility
depends on how RecipientIcon renders its output — but that's existing behavior,
not introduced by this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
