Adds Decorative and Informative Icon support in terra-button #3640

supreethmr · 2022-07-07T20:35:58Z

Summary

New sub component terra-icon-button has been created to display icon. Terra-Icon-button has two required props icon and iconType. text prop is optional since the subcomponent is best suitable to be used when we need to display icon or text with icon.

Breaking Changes :
icon, isIconOnly and isReversed prop are removed from terra-button. terra-default button will no longer support displaying icon and requires new sub-component to be used for icon buttons.

Terra-Icon has now made a11yLabel a required prop. this change requires update in all the components that are consuming terra-icon MVB.

This PR updates all the components to consume terra-icon MVB changes.

Closes #

Deployment Link

https://terra-core-deployed-pr-#.herokuapp.com/

Testing

Jest Tests were added to validate terra-icon-button for the prop changes.

Additional Details

Thank you for contributing to Terra.
@cerner/terra

benbcai · 2022-07-08T19:00:59Z

packages/terra-alert/src/Alert.jsx

@@ -89,19 +89,19 @@ const defaultProps = {
 const getAlertIcon = (type, customIcon) => {
  switch (type) {
    case AlertTypes.ALERT:
-      return (<span className={cx('icon')}><IconAlert /></span>);
+      return (<span className={cx('icon')}><IconAlert a11yLabel="Alert" /></span>);


Do we need to localized this a11y label? I can't find a11yLabel in terra-icon so not sure how it is used.

a11yLabel is a new prop that was added as part of the accessibility upgrade:
https://github.com/cerner/terra-core/blob/64b6b9fc41d741160887028e6b42155c51d40887/packages/terra-icon/src/IconBase.jsx

Ah, it's in IconBase. Thanks for the reference.

So if this will create a title element with this label and be read by the screen reader, I wonder if it does need to be localized. Getting the translations for all of these can be tedious.

updated to include translated messages for the terra-search-field and terra-alert which are having translations messages already.
b6652a1

For the other components updated the links to consume decorative icons. we might need to verify all the components that consumes terra-icon to find out the type of icon that should be used to make component accessible.

saket2403 · 2022-07-12T09:54:29Z

packages/terra-button/src/variants/_IconButton.jsx

+  /**
+   * Sets the button variant. One of `action` or `utility`.
+   */
+  variant: PropTypes.oneOf([ButtonVariants.ACTION, ButtonVariants.UTILITY]),


Should this list also contain neutral as default variant is neutral. Or should the default variant be one of action/utility?

I missed adding back the variants. IconButton supports all the variants.
Added here

ShettyAkarsh · 2022-07-12T13:56:00Z

should this changes be released as MVB ?

ShettyAkarsh · 2022-07-12T13:59:18Z

packages/terra-button/src/variants/_IconButton.jsx

+  /**
+   * Whether or not the button should only display as an icon.
+   */
+  isIconOnly: PropTypes.bool,


do we need this prop now ? as this file is specific to icon button

yes we would still need this because icon-button supports both icon and icon with text variants.

supreethmr · 2022-07-12T17:32:16Z

should this changes be released as MVB ?

yes, this changes should be released as MVB since default Button API no longer supports Icons.

Added upgrade guide

benbcai · 2022-07-15T14:17:37Z

packages/terra-button-group/src/ButtonGroupButton.jsx

+          icon={icon}
+          iconType={iconType}
+          isDisabled={isDisabled}
+          isIconOnly={icon != null}


isIconOnly should always be true here.

benbcai · 2022-07-15T14:23:03Z

packages/terra-button/CHANGELOG.md

+  * Added new subcomponent `terra-icon-button` to display icon.
+
+* Breaking Changes
+  * `icon`, `isIconOnly` and `isReveresed` props have been moved from `terra-button` to `terra-icon-button`.


terra-icon-button sounds like a package name. Should we be referencing it as IconButton instead?

benbcai · 2022-07-15T14:28:11Z

packages/terra-button/src/Button.jsx

   */
  text: PropTypes.string.isRequired,
  /**
-   * Additional information to display as a native tooltip on hover.
-   * Buttons declared as `isIconOnly` or `utility` will fallback to using `text` if not provided.
+   * Additional information to display as a native tooltip on hover. will fallback to using `text` if not provided.


Suggested change

* Additional information to display as a native tooltip on hover. will fallback to using `text` if not provided.

* Additional information to display as a native tooltip on hover. It will fallback to using `text` if not provided.

benbcai · 2022-07-15T14:41:55Z

packages/terra-button/src/variants/_IconButton.jsx

+  /**
+   * Sets the button variant. One of `action` or `utility`.
+   */
+  variant: PropTypes.oneOf([ButtonVariants.NEUTRAL, ButtonVariants.EMPHASIS, ButtonVariants.GHOST, ButtonVariants['DE-EMPHASIS'], ButtonVariants.ACTION]),


Should ButtonVariants.UTILITY also be one of the variants for IconButton?

I missed it while doing copy paste from button updated

benbcai · 2022-07-15T14:48:42Z

packages/terra-alert/CHANGELOG.md

@@ -4,6 +4,7 @@

 * Changed
  * Updated jest snapshots.
+  * Updated jest snapshots.


Should the change log reflex what is really changed in Alert.jsx which is adding a11yLabel to the icons for the alerts?

Updated here

benbcai · 2022-07-15T14:54:25Z

packages/terra-core-docs/src/terra-dev-site/doc/button/UpgradeGuide.3.doc.mdx

@@ -4,6 +4,13 @@ import { Badge } from 'terra-button/package.json?dev-site-package';

 # Terra Button Upgrade Guide

+## Changes from version 3 to version 4
+* Added new sub-component terra-icon-button to display decorative and informative icons within button.


Suggested change

* Added new sub-component terra-icon-button to display decorative and informative icons within button.

* Added new sub-component IconButton to display decorative and informative icons within button.

benbcai · 2022-07-15T14:55:14Z

packages/terra-core-docs/src/terra-dev-site/doc/button/UpgradeGuide.3.doc.mdx

+* `icon`, `isIconOnly` and `isReversed` props have been moved to terra-icon-button.
+* `Utility` variant have been moved to terra-icon-button.


Suggested change

* `icon`, `isIconOnly` and `isReversed` props have been moved to terra-icon-button.

* `Utility` variant have been moved to terra-icon-button.

* `icon`, `isIconOnly` and `isReversed` props have been moved to IconButton.

* `Utility` variant have been moved to IconButton.

benbcai · 2022-07-15T14:57:08Z

packages/terra-core-docs/src/terra-dev-site/doc/button/example/ButtonIcon.jsx

-    <Button icon={Icon} text="icon" isReversed className={cx('button')} />
-    <Button icon={Icon} isIconOnly text="Icon Only Button" className={cx('button')} />
+    <IconButton icon={Icon} text="icon" iconType={IconTypes.INFORMATIVE} className={cx('button')} />
+    <IconButton icon={Icon} text="icon" iconType="informative" isReversed className={cx('button')} />


Suggested change

<IconButton icon={Icon} text="icon" iconType="informative" isReversed className={cx('button')} />

<IconButton icon={Icon} text="icon" iconType={IconTypes.INFORMATIVE} isReversed className={cx('button')} />

* Added Decorative and Informative support in terra-button * jest snapshot updates * lint error * trailing space removed * change logs added to make terra-bot happy * snapshot update * screenshots updates for alert due to changes in icon style * searchfeild snapshots * added translations for alert searchfield * linter * scrreenshot updates * Added prop types * change log updates to rename terra-icon-button to iconbutton * minor change * minnor doc and jest test corrections * Add Icon Button to A11y Guide * Update A11yButtonLabel.jsx Co-authored-by: saket2403 <saket.bajaj1998@gmail.com> Co-authored-by: Saket Bajaj <42207428+saket2403@users.noreply.github.com>

Added Decorative and Informative support in terra-button

e0f94bf

supreethmr requested a review from a team as a code owner July 7, 2022 20:35

supreethmr added 3 commits July 8, 2022 14:16

jest snapshot updates

83d7bd4

lint error

8be8676

trailing space removed

f2a2b72

supreethmr requested a review from saket2403 July 8, 2022 10:18

supreethmr added 3 commits July 8, 2022 16:11

change logs added to make terra-bot happy

33afdd4

snapshot update

70e85e1

screenshots updates for alert due to changes in icon style

eb4da63

benbcai reviewed Jul 8, 2022

View reviewed changes

supreethmr added 4 commits July 9, 2022 23:55

searchfeild snapshots

6ab0eb1

added translations for alert searchfield

b6652a1

linter

2d4ede6

scrreenshot updates

f9652ca

saket2403 reviewed Jul 12, 2022

View reviewed changes

Added prop types

e98c61f

ShettyAkarsh reviewed Jul 12, 2022

View reviewed changes

saket2403 approved these changes Jul 15, 2022

View reviewed changes

benbcai reviewed Jul 15, 2022

View reviewed changes

supreethmr added 2 commits July 15, 2022 20:46

change log updates to rename terra-icon-button to iconbutton

c5a7356

minor change

c402015

minnor doc and jest test corrections

7d015f5

benbcai approved these changes Jul 15, 2022

View reviewed changes

ShettyAkarsh approved these changes Jul 18, 2022

View reviewed changes

supreethmr and others added 3 commits July 18, 2022 12:12

Merge branch 'A11y-MVB' into button-icon-support

f0329a5

Add Icon Button to A11y Guide

ae74e31

Update A11yButtonLabel.jsx

49785c4

supreethmr merged commit 8b6e77d into A11y-MVB Jul 18, 2022

supreethmr deleted the button-icon-support branch July 18, 2022 14:46

saket2403 restored the button-icon-support branch July 18, 2022 14:48

supreethmr mentioned this pull request Feb 9, 2023

[terra-button] Added support for Decorative Icon buttons and Anchor button #3722

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds Decorative and Informative Icon support in terra-button #3640

Adds Decorative and Informative Icon support in terra-button #3640

supreethmr commented Jul 7, 2022 •

edited

Loading

benbcai Jul 8, 2022

sdadn Jul 8, 2022

benbcai Jul 8, 2022

supreethmr Jul 11, 2022

saket2403 Jul 12, 2022

supreethmr Jul 12, 2022

ShettyAkarsh commented Jul 12, 2022

ShettyAkarsh Jul 12, 2022

supreethmr Jul 12, 2022

supreethmr commented Jul 12, 2022 •

edited

Loading

benbcai Jul 15, 2022

supreethmr Jul 15, 2022

benbcai Jul 15, 2022

supreethmr Jul 15, 2022

benbcai Jul 15, 2022

supreethmr Jul 15, 2022

benbcai Jul 15, 2022

supreethmr Jul 15, 2022

benbcai Jul 15, 2022

supreethmr Jul 15, 2022

benbcai Jul 15, 2022

supreethmr Jul 15, 2022

benbcai Jul 15, 2022

supreethmr Jul 15, 2022

supreethmr Jul 15, 2022

benbcai Jul 15, 2022

supreethmr Jul 15, 2022

	* Additional information to display as a native tooltip on hover. will fallback to using `text` if not provided.
	* Additional information to display as a native tooltip on hover. It will fallback to using `text` if not provided.

	* Added new sub-component terra-icon-button to display decorative and informative icons within button.
	* Added new sub-component IconButton to display decorative and informative icons within button.

		* `icon`, `isIconOnly` and `isReversed` props have been moved to terra-icon-button.
		* `Utility` variant have been moved to terra-icon-button.

	<IconButton icon={Icon} text="icon" iconType="informative" isReversed className={cx('button')} />
	<IconButton icon={Icon} text="icon" iconType={IconTypes.INFORMATIVE} isReversed className={cx('button')} />

Adds Decorative and Informative Icon support in terra-button #3640

Adds Decorative and Informative Icon support in terra-button #3640

Conversation

supreethmr commented Jul 7, 2022 • edited Loading

Summary

Deployment Link

Testing

Additional Details

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ShettyAkarsh commented Jul 12, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

supreethmr commented Jul 12, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

supreethmr commented Jul 7, 2022 •

edited

Loading

supreethmr commented Jul 12, 2022 •

edited

Loading