Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
3 changes: 3 additions & 0 deletions translate/public/locale/en-US/translate.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,9 @@ entitydetails-ContextIssueButton--context-issue-button = REQUEST CONTEXT or REPO
entitieslist-Entity--sibling-strings-title =
.title = Click to reveal sibling strings

entitieslist-Entity--listitem-label =
.aria-label = Select "{ $original }" for translation.

entitieslist-EntitiesList--clear-selected = <glyph></glyph>CLEAR
.title = Uncheck selected strings

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,11 @@ describe('<EntitiesList>', () => {
history,
);

expect(getAllByRole('listitem')).toHaveLength(2);
expect(
getAllByRole('button', {
name: 'Select "{ $original }" for translation.',
}),
).toHaveLength(2);
});

// FIXME: https://github.com/mozilla/pontoon/issues/3883
Expand Down
72 changes: 38 additions & 34 deletions translate/src/modules/entitieslist/components/Entity.test.jsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import { mount, shallow } from 'enzyme';
import React from 'react';

import * as hookModule from '~/hooks/useTranslator';
import { Entity } from './Entity';
import { vitest } from 'vitest';
import { it, vitest } from 'vitest';
import { fireEvent, render } from '@testing-library/react';
import { MockLocalizationProvider } from '~/test/utils';

beforeAll(() => {
vitest.mock('~/hooks/useTranslator', () => ({
Expand Down Expand Up @@ -61,91 +62,94 @@ describe('<Entity>', () => {
warnings: ['warning'],
},
};
const WrapEntity = (props) => {
return (
<MockLocalizationProvider>
<Entity {...props} />
</MockLocalizationProvider>
);
};

it('renders the source string and the first translation', () => {
const wrapper = shallow(<Entity entity={ENTITY_A} parameters={{}} />);

const contents = wrapper.find('Translation');
expect(contents.first().props().content).toContain(ENTITY_A.original);
expect(contents.last().props().content).toContain(
ENTITY_A.translation.string,
const { getByText } = render(
<WrapEntity entity={ENTITY_A} parameters={{}} />,
);
});

it('shows the correct status class', () => {
let wrapper = shallow(<Entity entity={ENTITY_A} parameters={{}} />);
expect(wrapper.find('.approved')).toHaveLength(1);

wrapper = shallow(<Entity entity={ENTITY_B} parameters={{}} />);
expect(wrapper.find('.pretranslated')).toHaveLength(1);

wrapper = shallow(<Entity entity={ENTITY_C} parameters={{}} />);
expect(wrapper.find('.missing')).toHaveLength(1);
getByText(ENTITY_A.original);
getByText(ENTITY_A.translation.string);
});

wrapper = shallow(<Entity entity={ENTITY_D} parameters={{}} />);
expect(wrapper.find('.errors')).toHaveLength(1);
it.each([
{ entity: ENTITY_A, classname: 'approved' },
{ entity: ENTITY_B, classname: 'pretranslated' },
{ entity: ENTITY_C, classname: 'missing' },
{ entity: ENTITY_D, classname: 'errors' },
{ entity: ENTITY_E, classname: 'warnings' },
])('renders $classname state correctly', ({ entity, classname }) => {
const { container } = render(
<WrapEntity entity={entity} parameters={{}} />,
);

wrapper = shallow(<Entity entity={ENTITY_E} parameters={{}} />);
expect(wrapper.find('.warnings')).toHaveLength(1);
expect(container.querySelector(`.${classname}`)).toBeInTheDocument();
});

it('calls the selectEntity function on click on li', () => {
const selectEntityFn = vi.fn();
const wrapper = mount(
<Entity
const { getByRole } = render(
<WrapEntity
entity={ENTITY_A}
selectEntity={selectEntityFn}
parameters={{}}
/>,
);
wrapper.find('li').simulate('click');
fireEvent.click(getByRole('button'));
expect(selectEntityFn).toHaveBeenCalledOnce();
});

it('calls the toggleForBatchEditing function on click on .status', () => {
hookModule.useTranslator.mockReturnValue(true);
const toggleForBatchEditingFn = vi.fn();
const wrapper = mount(
<Entity
const { getByRole } = render(
<WrapEntity
entity={ENTITY_A}
isReadOnlyEditor={false}
toggleForBatchEditing={toggleForBatchEditingFn}
parameters={{}}
/>,
);
wrapper.find('.status').simulate('click');
fireEvent.click(getByRole('checkbox'));
expect(toggleForBatchEditingFn).toHaveBeenCalledOnce();
});

it('does not call the toggleForBatchEditing function if user not translator', () => {
const toggleForBatchEditingFn = vi.fn();
const selectEntityFn = vi.fn();
const wrapper = mount(
<Entity
const { getByRole } = render(
<WrapEntity
entity={ENTITY_A}
isReadOnlyEditor={false}
toggleForBatchEditing={toggleForBatchEditingFn}
selectEntity={selectEntityFn}
parameters={{}}
/>,
);
wrapper.find('.status').simulate('click');
fireEvent.click(getByRole('checkbox'));
expect(toggleForBatchEditingFn).not.toHaveBeenCalled();
});

it('does not call the toggleForBatchEditing function if read-only editor', () => {
const toggleForBatchEditingFn = vi.fn();
const selectEntityFn = vi.fn();
const wrapper = mount(
<Entity
const { getByRole } = render(
<WrapEntity
entity={ENTITY_A}
isReadOnlyEditor={true}
toggleForBatchEditing={toggleForBatchEditingFn}
selectEntity={selectEntityFn}
parameters={{}}
/>,
);
wrapper.find('.status').simulate('click');
fireEvent.click(getByRole('checkbox'));
expect(toggleForBatchEditingFn).not.toHaveBeenCalled();
});
});
96 changes: 56 additions & 40 deletions translate/src/modules/entitieslist/components/Entity.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -113,47 +113,63 @@ export function Entity({
);

return (
<li className={cn} onClick={handleSelectEntity}>
<span className='status fas' onClick={handleForBatchEditing} />
{selected && !entity.isSibling ? (
<Localized
id='entitieslist-Entity--listitem-label'
attrs={{ 'aria-label': true }}
vars={{ original: entity.original }}
>
<li
className={cn}
role='button'
aria-label={'Select "{ $original }" for translation.'}
Copy link
Member

Choose a reason for hiding this comment

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

Probably best to leave this backup assignment out, esp. given the $original placeholder. The formatted value should be getting overridden by the <Localized> in any case.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ohk, removed the fallback aria-label

onClick={handleSelectEntity}
>
<span
className='status fas'
role='checkbox'
aria-selected={checkedForBatchEditing}
onClick={handleForBatchEditing}
/>
{selected && !entity.isSibling ? (
<div>
{!areSiblingsActive && showSiblingEntitiesButton() && (
<Localized id='entitieslist-Entity--sibling-strings-title'>
<i
className={'sibling-entities-icon fas fa-expand-arrows-alt'}
title='Click to reveal sibling strings'
onClick={showSiblingEntities}
></i>
</Localized>
)}
</div>
) : null}
<div>
{!areSiblingsActive && showSiblingEntitiesButton() && (
<Localized id='entitieslist-Entity--sibling-strings-title'>
<i
className={'sibling-entities-icon fas fa-expand-arrows-alt'}
title='Click to reveal sibling strings'
onClick={showSiblingEntities}
></i>
</Localized>
)}
<p className='source-string'>
<Translation
content={entity.original}
format={entity.format}
search={
parameters.search_exclude_source_strings
? null
: parameters.search
}
/>
</p>
<p
className='translation-string'
dir={direction}
lang={code}
data-script={script}
>
<Translation
content={entity.translation?.string ?? ''}
format={entity.format}
search={parameters.search}
/>
</p>
<div className='indicator fas fa-chevron-right'></div>
</div>
) : null}
<div>
<p className='source-string'>
<Translation
content={entity.original}
format={entity.format}
search={
parameters.search_exclude_source_strings
? null
: parameters.search
}
/>
</p>
<p
className='translation-string'
dir={direction}
lang={code}
data-script={script}
>
<Translation
content={entity.translation?.string ?? ''}
format={entity.format}
search={parameters.search}
/>
</p>
<div className='indicator fas fa-chevron-right'></div>
</div>
</li>
</li>
</Localized>
);
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import ftl from '@fluent/dedent';
import { shallow } from 'enzyme';
import React from 'react';
import { parseEntry } from '~/utils/message';
import { FluentAttribute } from './FluentAttribute';
import { render } from '@testing-library/react';
import { MockLocalizationProvider } from '~/test/utils';

describe('isSimpleSingleAttributeMessage', () => {
it('renders nonempty for a string with a single attribute', () => {
Expand All @@ -11,8 +12,12 @@ describe('isSimpleSingleAttributeMessage', () => {
.an-atribute = Hello!
`;
const entry = parseEntry('fluent', original);
const wrapper = shallow(<FluentAttribute entry={entry} />);
expect(wrapper.isEmptyRender()).toEqual(false);
const { container } = render(
<MockLocalizationProvider>
<FluentAttribute entry={entry} />
</MockLocalizationProvider>,
);
expect(container).not.toBeEmptyDOMElement();
});

it('renders null for string with value', () => {
Expand All @@ -21,8 +26,8 @@ describe('isSimpleSingleAttributeMessage', () => {
.an-atribute = Hello!
`;
const entry = parseEntry('fluent', original);
const wrapper = shallow(<FluentAttribute entry={entry} />);
expect(wrapper.isEmptyRender()).toEqual(true);
const { container } = render(<FluentAttribute entry={entry} />);
expect(container).toBeEmptyDOMElement();
});

it('renders null for string with several attributes', () => {
Expand All @@ -32,7 +37,7 @@ describe('isSimpleSingleAttributeMessage', () => {
.two-attrites = World!
`;
const entry = parseEntry('fluent', original);
const wrapper = shallow(<FluentAttribute entry={entry} />);
expect(wrapper.isEmptyRender()).toEqual(true);
const { container } = render(<FluentAttribute entry={entry} />);
expect(container).toBeEmptyDOMElement();
});
});
Loading