-
Notifications
You must be signed in to change notification settings - Fork 165
Conversation
packages/terra-core-docs/src/terra-dev-site/doc/badge/example/BadgeIntent.jsx
Outdated
Show resolved
Hide resolved
packages/terra-core-docs/src/terra-dev-site/doc/badge/example/BadgeIntent.jsx
Outdated
Show resolved
Hide resolved
@MadanKumarGovindaswamy I have reviewed the updated content and will have edits ready near end of the day tomorrow or Thursday morning. |
I reviewed the most recent updates to the implementation page and feel great about the changes helping consumers to implement more accessible content. Thank you for your hard work, @MadanKumarGovindaswamy ! |
@@ -2,6 +2,9 @@ | |||
|
|||
## Unreleased | |||
|
|||
* Changed | |||
* Update props description in props table. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this PR has more changes than just prop description updates in the terra badge, can you also add logs for the src code changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated . Thanks
Commit Link -- b7717dd
@@ -98,9 +106,8 @@ const Badge = ({ | |||
); | |||
|
|||
const textContent = text ? <span className={styles.text}>{text}</span> : null; | |||
const intentText = intent !== BadgeIntent.DEFAULT ? <VisuallyHiddenText text={intl.formatMessage({ id: `Terra.badge.intent.${intent}` })} /> : null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would like to know why intentText
is removed ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @supreethmr ,
This has been updated after Scott comment in the jira.
-- The badge intent should be the words themselves inside the badge. The words Primary, Secondary, Positive, and Negative will most likely not be the context intended for the user to understand. And it should not be only available to screen reader users.
Thanks
Summary
What was changed:
Intent badge example has been updated.
Why it was changed:
Existing example does not provide much context how to use badge.
Testing
This change was tested using:
Reviews
In addition to engineering reviews, this PR needs:
Additional Details
This PR resolves:
UXPLATFORM-XXXX
Thank you for contributing to Terra.
@cerner/terra