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.

Sign up for GitHub

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

[BD-46] feat: add i18n support for ProductTour Checkpoint sr-only message #2758

Merged
merged 2 commits into from
Nov 1, 2023
Merged

[BD-46] feat: add i18n support for ProductTour Checkpoint sr-only message #2758

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4446978
feat: add i18n support for sr-only message
Oct 27, 2023
ae63a66
fix: add fixes after review
Oct 31, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions src/ProductTour/Checkpoint.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import React, { useEffect, useState } from 'react';
import { useMediaQuery } from 'react-responsive';
import PropTypes from 'prop-types';
import { createPopper } from '@popperjs/core';
import { FormattedMessage } from 'react-intl';

import breakpoints from '../utils/breakpoints';

Expand Down Expand Up @@ -95,8 +96,13 @@ const Checkpoint = React.forwardRef(({
role="dialog"
style={{ visibility: checkpointVisible ? 'visible' : 'hidden', pointerEvents: checkpointVisible ? 'auto' : 'none' }}
>
{/* This text is not translated due to Paragon's lack of i18n support */}
<span className="sr-only">Top of step {index + 1}</span>
<span className="sr-only">
<FormattedMessage
id="pgn.checkpoint.sr-only.message"
defaultMessage={`Top of step ${index + 1}`}
Copy link
Contributor

Choose a reason for hiding this comment

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

Do I need to add keys for this translation to the i18n folder?

description="Screen-reader message to indicate the user's position in a sequence of checkpoints."
/>
Copy link
Contributor

Choose a reason for hiding this comment

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

This not the right way to translate messages with variables in it, the correct syntax is as follows (see docs):

<FormattedMessage
  id="pgn.ProductTour.Checkpoint.position-text" // should change the id as well, we try to follow <component>.<subComponent> pattern in our ids
  defaultMessage="Top of step {step}"
  value={{ step: index + 1 }}
  description="Screen-reader message to indicate the user's position in a sequence of checkpoints."
/>

</span>
{(title || !isOnlyCheckpoint) && (
<div className="pgn__checkpoint-header">
<CheckpointTitle>{title}</CheckpointTitle>
Expand Down
17 changes: 9 additions & 8 deletions src/ProductTour/Checkpoint.test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React from 'react';
import { render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { IntlProvider } from 'react-intl';

import * as popper from '@popperjs/core';

Expand All @@ -24,7 +25,7 @@ describe('Checkpoint', () => {
describe('second Checkpoint in Tour', () => {
beforeEach(() => {
render(
<>
<IntlProvider locale="en" messages={{}}>
<div id="target-element">...</div>
<Checkpoint
advanceButtonText="Next"
Expand All @@ -39,7 +40,7 @@ describe('Checkpoint', () => {
title="Checkpoint title"
totalCheckpoints={5}
/>
</>,
</IntlProvider>,
);
});

Expand Down Expand Up @@ -75,7 +76,7 @@ describe('Checkpoint', () => {
describe('last Checkpoint in Tour', () => {
beforeEach(() => {
render(
<>
<IntlProvider locale="en" messages={{}}>
<div id="#last-element" />
<Checkpoint
advanceButtonText="Next"
Expand All @@ -90,7 +91,7 @@ describe('Checkpoint', () => {
title="Checkpoint title"
totalCheckpoints={5}
/>
</>,
</IntlProvider>,
);
});

Expand All @@ -108,7 +109,7 @@ describe('Checkpoint', () => {
describe('only one Checkpoint in Tour', () => {
beforeEach(() => {
render(
<>
<IntlProvider locale="en" messages={{}}>
<div id="#target-element" />
<Checkpoint
advanceButtonText="Next"
Expand All @@ -123,7 +124,7 @@ describe('Checkpoint', () => {
title="Checkpoint title"
totalCheckpoints={1}
/>
</>,
</IntlProvider>,
);
});

Expand All @@ -140,7 +141,7 @@ describe('Checkpoint', () => {
describe('only one Checkpoint in Tour and showDismissButton set to true', () => {
it('it renders dismiss button and end button', () => {
render(
<>
<IntlProvider locale="en" messages={{}}>
<div id="#target-element" />
<Checkpoint
advanceButtonText="Next"
Expand All @@ -156,7 +157,7 @@ describe('Checkpoint', () => {
totalCheckpoints={1}
showDismissButton
/>
</>,
</IntlProvider>,
);
expect(screen.getByText('Dismiss', { selector: 'button' })).toBeInTheDocument();
expect(screen.getByText('End', { selector: 'button' })).toBeInTheDocument();
Expand Down
Loading
Loading