Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add badge #74

Closed
wants to merge 3 commits into from
Closed

add badge #74

wants to merge 3 commits into from

Conversation

yonashailug
Copy link
Contributor

No description provided.

@@ -0,0 +1,53 @@
// Copyright (c) Microsoft Corporation.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we also change the COVERAGE.md file?

@@ -0,0 +1,53 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might also need to run the update:eslint-docs command to update the README file.

// visitor functions for different types of nodes
JSXOpeningElement(node) {
// if it is not a Badge, return
if (elementType(node) !== "Badge") {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if we need two separate rules for the Badge component.

A badge will take the accessible name of the parent such as a Tooltip or a Button, and will need to further labelling in that case.

A badge containing text content will have an accessible name so there is no need for other labels.

If a badge has an icon, we must add aria-label or aria-labelledby UNLESS the Badge is marked as decorative with aria-hidden.

If a badge has no content or an icon and the color conveys meaning, we can add the meaning with an aria-label. It depends on the context.

Suggestion:
We can consolidate the two newly created rules into one and we can have code like this:

                // if it has a tooltip parent, return
                if (hasToolTipParent(context)) {
                    return;
                }

                // if it has text content, return
                if (hasTextContentChild(node)) {
                    return;
                }

                // the button has an associated label
                if (hasAssociatedLabelViaAriaLabelledBy(openingElement, context)) {
                    return;
                }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants