Skip to content
This repository has been archived by the owner on May 24, 2024. It is now read-only.

[terra-badge] Example Doc Update #3952

Merged
merged 17 commits into from
Nov 13, 2023
Merged

[terra-badge] Example Doc Update #3952

merged 17 commits into from
Nov 13, 2023

Conversation

MadanKumarGovindaswamy
Copy link
Contributor

@MadanKumarGovindaswamy MadanKumarGovindaswamy commented Oct 27, 2023

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:

  • WDIO
  • Jest
  • Visual testing (please attach a screenshot or recording)
  • Other (please describe below)
  • No tests are needed

Reviews

In addition to engineering reviews, this PR needs:

  • UX review
  • Accessibility review
  • Functional review

Additional Details

This PR resolves:

UXPLATFORM-XXXX


Thank you for contributing to Terra.
@cerner/terra

@cerner cerner deleted a comment from github-actions bot Oct 27, 2023
@supreethmr supreethmr marked this pull request as ready for review October 31, 2023 06:45
@supreethmr supreethmr requested a review from a team as a code owner October 31, 2023 06:45
@scottwilmarth
Copy link

@MadanKumarGovindaswamy I have reviewed the updated content and will have edits ready near end of the day tomorrow or Thursday morning.

@scottwilmarth scottwilmarth added the ⭐ UX Reviewed UX Reviewed and approved. label Nov 8, 2023
@scottwilmarth
Copy link

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.
Copy link
Contributor

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

Copy link
Contributor Author

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;
Copy link
Contributor

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 ?

Copy link
Contributor Author

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

@github-actions github-actions bot temporarily deployed to preview-pr-3952 November 9, 2023 07:31 Destroyed
@gt106551 gt106551 removed the ⭐ UX Reviewed UX Reviewed and approved. label Nov 10, 2023
@supreethmr supreethmr added the ⭐ UX Reviewed UX Reviewed and approved. label Nov 13, 2023
@sugan2416 sugan2416 merged commit 49733f7 into main Nov 13, 2023
22 checks passed
@sugan2416 sugan2416 deleted the Terra-Badge-Doc-Update branch November 13, 2023 08:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
📦 terra-badge ⭐ UX Reviewed UX Reviewed and approved.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants