Browse Source

feat(ui): Use fragment in translate instead of span (#41289)

switch to using fragments instead of spans in translated text, would
help with RTL by making text easier to select, the spans give us nothing
except more dom elements.
Scott Cooper 2 years ago
parent
commit
707b8b0b2d

+ 1 - 2
static/app/components/assigneeSelector.spec.jsx

@@ -369,8 +369,7 @@ describe('AssigneeSelector', () => {
     expect(screen.getByTestId('suggested-avatar-stack')).toBeInTheDocument();
     // Hover over avatar
     userEvent.hover(screen.getByTestId('letter_avatar-avatar'));
-    expect(await screen.findByText('Suggestion:')).toBeInTheDocument();
-    expect(screen.getByText('Jane Bloggs')).toBeInTheDocument();
+    expect(await screen.findByText('Suggestion: Jane Bloggs')).toBeInTheDocument();
     expect(screen.getByText('commit data')).toBeInTheDocument();
 
     await openMenu();

+ 2 - 1
static/app/components/events/eventAttachments.spec.tsx

@@ -33,7 +33,8 @@ describe('EventAttachments', function () {
 
     expect(
       screen.getByText(
-        'Your limit of stored crash reports has been reached for this issue.'
+        'Your limit of stored crash reports has been reached for this issue.',
+        {exact: false}
       )
     ).toBeInTheDocument();
   });

+ 2 - 2
static/app/components/events/eventEntries.spec.tsx

@@ -199,7 +199,7 @@ describe('EventEntries', function () {
 
           expect(errorItem.length).toBe(1);
           expect(
-            screen.getByText('Some frames appear to be minified. Did you configure the')
+            screen.getByText('Some frames appear to be minified. Did you configure the ?')
           ).toBeInTheDocument();
 
           expect(
@@ -277,7 +277,7 @@ describe('EventEntries', function () {
 
           expect(errorItem.length).toBe(1);
           expect(
-            screen.getByText('Some frames appear to be minified. Did you configure the')
+            screen.getByText('Some frames appear to be minified. Did you configure the ?')
           ).toBeInTheDocument();
 
           expect(screen.getByText('Sentry Gradle Plugin')).toBeInTheDocument();

+ 3 - 2
static/app/components/group/externalIssueForm.spec.jsx

@@ -181,8 +181,9 @@ describe('ExternalIssueForm', () => {
       it('fast typing is debounced and uses trailing call when fetching data', async () => {
         renderComponent('Link');
         jest.useFakeTimers();
-        userEvent.click(screen.getAllByText('Issue').at(1));
-        userEvent.type(screen.getByRole('textbox', {name: 'Issue'}), 'faster');
+        const textbox = screen.getByRole('textbox', {name: 'Issue'});
+        userEvent.click(textbox);
+        userEvent.type(textbox, 'faster');
         expect(window.fetch).toHaveBeenCalledTimes(0);
         jest.advanceTimersByTime(300);
         expect(window.fetch).toHaveBeenCalledTimes(1);

+ 1 - 4
static/app/components/group/inboxBadges/inboxReason.spec.jsx

@@ -52,9 +52,6 @@ describe('InboxReason', () => {
     const tag = screen.getByText('Unignored');
     userEvent.hover(tag);
 
-    // Text is split up because of translations
-    expect(await screen.findByText('Affected')).toBeInTheDocument();
-    expect(screen.getByText('10')).toBeInTheDocument();
-    expect(screen.getByText('user(s)')).toBeInTheDocument();
+    expect(await screen.findByText('Affected 10 user(s)')).toBeInTheDocument();
   });
 });

+ 8 - 2
static/app/components/modals/emailVerificationModal.spec.tsx

@@ -17,7 +17,10 @@ describe('Email Verification Modal', function () {
       />,
       {context: routerContext}
     );
-    const message = screen.getByText('Please verify your email before');
+    const message = screen.getByText(
+      'Please verify your email before taking this action',
+      {exact: false}
+    );
     expect(message.parentElement).toHaveTextContent(
       'Please verify your email before taking this action, or go to your email settings.'
     );
@@ -42,7 +45,10 @@ describe('Email Verification Modal', function () {
         actionMessage={actionMessage}
       />
     );
-    const message = screen.getByText('Please verify your email before');
+    const message = screen.getByText(
+      'Please verify your email before accepting the tenet',
+      {exact: false}
+    );
     expect(message.parentElement).toHaveTextContent(
       `Please verify your email before ${actionMessage}, or go to your email settings.`
     );

+ 29 - 0
static/app/locale.spec.tsx

@@ -4,6 +4,35 @@ import {textWithMarkupMatcher} from 'sentry-test/utils';
 import {tct} from 'sentry/locale';
 
 describe('locale.gettextComponentTemplate', () => {
+  it('should not wrap translated text in span', () => {
+    // spaces are removed because pretter keeps trying to remove them in the snapshot
+    expect(
+      tct('hello[one]two[three:3]', {
+        one: ' one',
+        three: <code />,
+      })
+    ).toMatchInlineSnapshot(`
+      <React.Fragment>
+        <React.Fragment>
+          <React.Fragment>
+            hello
+          </React.Fragment>
+          <React.Fragment>
+             one
+          </React.Fragment>
+          <React.Fragment>
+            two
+          </React.Fragment>
+          <code>
+            <React.Fragment>
+              3
+            </React.Fragment>
+          </code>
+        </React.Fragment>
+      </React.Fragment>
+    `);
+  });
+
   it('should render two component templates inside the same parent', () => {
     render(
       <div>

+ 5 - 5
static/app/locale.tsx

@@ -114,7 +114,7 @@ function formatForReact(formatString: string, args: FormatArg[]): React.ReactNod
       // array with two items in.
       match[2] = null;
       match[1] = 1;
-      nodes.push(<span key={idx++}>{sprintf.format([match], [null, arg])}</span>);
+      nodes.push(<Fragment key={idx++}>{sprintf.format([match], [null, arg])}</Fragment>);
     }
   });
 
@@ -241,7 +241,7 @@ export function renderTemplate(
 
     for (const item of group) {
       if (isString(item)) {
-        children.push(<span key={idx++}>{item}</span>);
+        children.push(<Fragment key={idx++}>{item}</Fragment>);
       } else {
         children.push(renderGroup(item.group));
       }
@@ -249,10 +249,10 @@ export function renderTemplate(
 
     // In case we cannot find our component, we call back to an empty
     // span so that stuff shows up at least.
-    let reference = components[groupKey] ?? <span key={idx++} />;
+    let reference = components[groupKey] ?? <Fragment key={idx++} />;
 
     if (!isValidElement(reference)) {
-      reference = <span key={idx++}>{reference}</span>;
+      reference = <Fragment key={idx++}>{reference}</Fragment>;
     }
 
     const element = reference as React.ReactElement;
@@ -281,7 +281,7 @@ function mark(node: React.ReactNode): string {
   // require some understanding of reacts internal types.
   const proxy = {
     $$typeof: Symbol.for('react.element'),
-    type: 'span',
+    type: Symbol.for('react.fragment'),
     key: null,
     ref: null,
     props: {

+ 1 - 2
static/app/utils/dashboards/issueFieldRenderers.spec.tsx

@@ -104,8 +104,7 @@ describe('getIssueFieldRenderer', function () {
       );
       expect(screen.getByText('TU')).toBeInTheDocument();
       userEvent.hover(screen.getByText('TU'));
-      expect(await screen.findByText('Assigned to')).toBeInTheDocument();
-      expect(screen.getByText('Test User')).toBeInTheDocument();
+      expect(await screen.findByText('Assigned to Test User')).toBeInTheDocument();
       expect(screen.getByText('Based on')).toBeInTheDocument();
       expect(screen.getByText('commit data')).toBeInTheDocument();
     });

+ 3 - 1
static/app/views/alerts/create.spec.jsx

@@ -494,7 +494,9 @@ describe('ProjectAlertsCreate', function () {
         );
       });
       expect(
-        screen.getByText('issues would have triggered this rule in the past 14 days')
+        screen.getByText('4 issues would have triggered this rule in the past 14 days', {
+          exact: false,
+        })
       ).toBeInTheDocument();
       for (const group of groups) {
         expect(screen.getByText(group.shortId)).toBeInTheDocument();

Some files were not shown because too many files changed in this diff