Skip to content

Commit

Permalink
fix: don't use role in badge/button prop (#449)
Browse files Browse the repository at this point in the history
Role is a reserved ARIA attribute. We already refer to the `Button` property as `modifier` though the PropTypes don't reflect that.

Closes #448
  • Loading branch information
burnumd committed Aug 29, 2024
1 parent 124693b commit 2a94bd6
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 11 deletions.
2 changes: 1 addition & 1 deletion src/components/Button/Button.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ Button.propTypes = {
/* Buttons can be either default or small, which affects their styling */
size: PropTypes.oneOf(["small"]),
/* Buttons can be either default or secondary, which affects their styling */
role: PropTypes.oneOf(["secondary"]),
modifier: PropTypes.oneOf(["secondary"]),
innerRef: PropTypes.any,
/** A unique identifier for the button */
id: PropTypes.string,
Expand Down
20 changes: 12 additions & 8 deletions src/components/PageContent/Badge/Badge.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,17 @@ import * as Rivet from "../../util/Rivet";

const badgeClass = "rvt-badge";

const computeStyle = (variant, role) => {
const computeStyle = (variant, modifier) => {
if (!variant || variant === "plain") {
if (role) {
return `${badgeClass}--${role}`;
if (modifier) {
return `${badgeClass}--${modifier}`;
} else {
return null;
}
} else {
let variantClass = `${badgeClass}--${variant}`;
if (role) {
variantClass += `-${role}`;
if (modifier) {
variantClass += `-${modifier}`;
}

return variantClass;
Expand All @@ -30,13 +30,17 @@ const Badge = ({
children,
className,
id = Rivet.shortuid(),
role,
type: modifier,
variant,
...attrs
}) => (
<span
id={id}
className={classNames(badgeClass, computeStyle(variant, role), className)}
className={classNames(
badgeClass,
computeStyle(variant, modifier),
className
)}
{...attrs}
>
{children}
Expand All @@ -48,7 +52,7 @@ Badge.propTypes = {
/* The variant determines the style of the badge */
variant: PropTypes.oneOf(["danger", "info", "plain", "success", "warning"]),
/* Badges can be either default or secondary, which affects their styling */
role: PropTypes.oneOf(["secondary"]),
modifier: PropTypes.oneOf(["secondary"]),
/** A unique identifier for the badge */
id: PropTypes.string,
};
Expand Down
4 changes: 2 additions & 2 deletions src/components/PageContent/Badge/Badge.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ describe("<Badge />", () => {

it("should render the appropriate role", () => {
render(
<Badge role="secondary" data-testid={badgeTestId}>
<Badge type="secondary" data-testid={badgeTestId}>
Secondary
</Badge>
);
Expand All @@ -40,7 +40,7 @@ describe("<Badge />", () => {

it("should render combinations of variant and role", () => {
render(
<Badge variant="success" role="secondary" data-testid={badgeTestId}>
<Badge variant="success" type="secondary" data-testid={badgeTestId}>
Success secondary
</Badge>
);
Expand Down

0 comments on commit 2a94bd6

Please sign in to comment.