-
Notifications
You must be signed in to change notification settings - Fork 81
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
feat: parametrized StepIndicator strings #2707
Changes from all commits
66df7e5
adbdb34
91d5c75
69b791c
51fe363
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,6 +16,8 @@ type StepIndicatorProps = { | |
HTMLHeadingElement | ||
> | ||
headingLevel: HeadingLevel | ||
stepText?: string | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If there are more of these, would it be better to do an object with multiple entries?
That way we can extend without adding loads more props? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These are the only two strings on the top-level component, so would it still be beneficial to put it in such an object? The other strings are on the StepIndicatorStep sub-component, which could use such a treatment, but I don't think we have that pattern on other components (yet) and wanted to keep this PR tight enough that we can unblock the project requirement. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay no worries, that works. Lemme test. |
||
ofText?: string | ||
} | ||
export const StepIndicator = ( | ||
props: StepIndicatorProps | ||
|
@@ -30,6 +32,8 @@ export const StepIndicator = ( | |
listProps, | ||
headingProps, | ||
headingLevel, | ||
stepText = 'Step', | ||
ofText = 'of', | ||
} = props | ||
|
||
const Heading = headingLevel | ||
|
@@ -84,12 +88,14 @@ export const StepIndicator = ( | |
<div className="usa-step-indicator__header"> | ||
<Heading className={headingClasses} {...remainingHeadingProps}> | ||
<span className="usa-step-indicator__heading-counter"> | ||
<span className="usa-sr-only">Step</span> | ||
<span className="usa-sr-only" data-testid="step-text"> | ||
{stepText} | ||
</span> | ||
<span className="usa-step-indicator__current-step"> | ||
{currentStepNumber} | ||
</span> | ||
| ||
<span className="usa-step-indicator__total-steps">{`of ${totalNumberOfSteps}`}</span> | ||
<span className="usa-step-indicator__total-steps">{`${ofText} ${totalNumberOfSteps}`}</span> | ||
| ||
</span> | ||
<span className="usa-step-indicator__heading-text"> | ||
|
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.
Recommend removing the passed in text in one of the tests so that we can ensure the fallback to english works correctly.
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.
added