Bug 1561091 - Refactor getFormattedMessage to set fluent attributes or use plain text (#5127)

content-src/components/CollapsibleSection/CollapsibleSection.jsx

@@ -1,5 +1,6 @@
 import {actionCreators as ac} from "common/Actions.jsm";
 import {ErrorBoundary} from "content-src/components/ErrorBoundary/ErrorBoundary";
+import {FluentOrText} from "content-src/components/FluentOrText/FluentOrText";
 import React from "react";
 import {SectionMenu} from "content-src/components/SectionMenu/SectionMenu";
 import {SectionMenuOptions} from "content-src/lib/section-menu-options";
@@ -7,18 +8,6 @@ import {SectionMenuOptions} from "content-src/lib/section-menu-options";
 const VISIBLE = "visible";
 const VISIBILITY_CHANGE_EVENT = "visibilitychange";
 
-function getFormattedMessage(message) {
-  return typeof message === "string" ? (
-    <span>{message}</span>
-  ) : (
-    <span
-      data-l10n-id={message.id}
-      data-l10n-args={
-        !message.values ? "{}" : `{ "provider": "${message.values.provider}" }`
-      } />
-  );
-}
-
 export class CollapsibleSection extends React.PureComponent {
   constructor(props) {
     super(props);
@@ -171,7 +160,7 @@ export class CollapsibleSection extends React.PureComponent {
             {/* Click-targets that toggle a collapsible section should have an aria-expanded attribute; see bug 1553234 */}
               <span className="click-target" role="button" tabIndex="0" onKeyPress={this.onKeyPress} onClick={this.onHeaderClick}>
                 {this.renderIcon()}
-                {getFormattedMessage(title)}
+                <FluentOrText message={title} />
               </span>
               <span className="click-target" role="button" tabIndex="0" onKeyPress={this.onKeyPress} onClick={this.onHeaderClick}>
                 {isCollapsible && <span className={`collapsible-arrow icon ${collapsed ? "icon-arrowhead-forward-small" : "icon-arrowhead-down-small"}`} />}
@@ -179,7 +168,9 @@ export class CollapsibleSection extends React.PureComponent {
               <span className="learn-more-link-wrapper">
                 {learnMore &&
                   <span className="learn-more-link">
-                    <a href={learnMore.link.href} data-l10n-id={learnMore.link.id}>{learnMore.link.text}</a>
+                    <FluentOrText message={learnMore.link.message}>
+                      <a href={learnMore.link.href} />
+                    </FluentOrText>
                   </span>
                 }
               </span>

content-src/components/DiscoveryStreamBase/DiscoveryStreamBase.jsx

@@ -217,7 +217,7 @@ export class _DiscoveryStreamBase extends React.PureComponent {
     const topSites = extractComponent("TopSites");
     const message = extractComponent("Message") || {
       header: {
-        link_id: topStories.learnMore.link.id,
+        link_text: topStories.learnMore.link.message,
         link_url: topStories.learnMore.link.href,
         title: topStories.title,
       },
@@ -240,8 +240,7 @@ export class _DiscoveryStreamBase extends React.PureComponent {
           learnMore={{
             link: {
               href: message.header.link_url,
-              text: message.header.link_text,
-              id: message.header.link_id
+              message: message.header.link_text,
             },
           }}
           privacyNoticeURL={topStories.privacyNoticeURL}

content-src/components/FluentOrText/FluentOrText.jsx

@@ -0,0 +1,32 @@
+import React from "react";
+
+/**
+ * Set text on a child element/component depending on if the message is already
+ * translated plain text or a fluent id with optional args.
+ */
+export class FluentOrText extends React.PureComponent {
+  render() {
+    // Ensure we have a single child to attach attributes
+    const { children, message } = this.props;
+    const child = children ? React.Children.only(children) : <span />;
+
+    // For a string message, just use it as the child's text
+    let grandChildren = message;
+    let extraProps;
+
+    // Convert a message object to set desired fluent-dom attributes
+    if (typeof message === "object") {
+      const args = message.args || message.values;
+      extraProps = {
+        "data-l10n-args": args && JSON.stringify(args),
+        "data-l10n-id": message.id || message.string_id,
+      };
+
+      // Use original children potentially with data-l10n-name attributes
+      grandChildren = child.props.children;
+    }
+
+    // Add the message to the child via fluent attributes or text node
+    return React.cloneElement(child, extraProps, grandChildren);
+  }
+}

content-src/components/Sections/Sections.jsx

@@ -2,6 +2,7 @@ import {actionCreators as ac, actionTypes as at} from "common/Actions.jsm";
 import {Card, PlaceholderCard} from "content-src/components/Card/Card";
 import {CollapsibleSection} from "content-src/components/CollapsibleSection/CollapsibleSection";
 import {ComponentPerfTimer} from "content-src/components/ComponentPerfTimer/ComponentPerfTimer";
+import {FluentOrText} from "content-src/components/FluentOrText/FluentOrText";
 import {connect} from "react-redux";
 import {MoreRecommendations} from "content-src/components/MoreRecommendations/MoreRecommendations";
 import {PocketLoggedInCta} from "content-src/components/PocketLoggedInCta/PocketLoggedInCta";
@@ -14,18 +15,6 @@ const VISIBILITY_CHANGE_EVENT = "visibilitychange";
 const CARDS_PER_ROW_DEFAULT = 3;
 const CARDS_PER_ROW_COMPACT_WIDE = 4;
 
-function getFormattedMessage(message) {
-  return typeof message === "string" ? (
-    <span>{message}</span>
-  ) : (
-    <span
-      data-l10n-id={message.id}
-      data-l10n-args={
-        !message.values ? "{}" : `{ "provider": "${message.values.provider}" }`
-      } />
-  );
-}
-
 export class Section extends React.PureComponent {
   get numRows() {
     const {rowsPref, maxRows, Prefs} = this.props;
@@ -249,9 +238,9 @@ export class Section extends React.PureComponent {
               {emptyState.icon && emptyState.icon.startsWith("moz-extension://") ?
                 <span className="empty-state-icon icon" style={{"background-image": `url('${emptyState.icon}')`}} /> :
                 <span className={`empty-state-icon icon icon-${emptyState.icon}`} />}
-              <p className="empty-state-message">
-                {getFormattedMessage(emptyState.message)}
-              </p>
+              <FluentOrText message={emptyState.message}>
+                <p className="empty-state-message" />
+              </FluentOrText>
             </div>
           </div>}
         {id === "topstories" &&

lib/SectionsManager.jsm

@@ -34,7 +34,7 @@ const BUILT_IN_SECTIONS = {
     learnMore: {
       link: {
         href: "https://getpocket.com/firefox/new_tab_learn_more",
-        id: "newtab-pocket-how-it-works",
+        message: {id: "newtab-pocket-how-it-works"},
       },
     },
     privacyNoticeURL: "https://www.mozilla.org/privacy/firefox/#suggest-relevant-content",

test/unit/content-src/components/FluentOrText.test.jsx

@@ -0,0 +1,66 @@
+import { FluentOrText } from "content-src/components/FluentOrText/FluentOrText";
+import React from "react";
+import { shallow } from "enzyme";
+
+describe("<FluentOrText>", () => {
+  it("should create span with no children", () => {
+    const wrapper = shallow(<FluentOrText />);
+
+    assert.ok(wrapper.find("span"));
+  });
+  it("should set plain text", () => {
+    const wrapper = shallow(<FluentOrText message={"hello"} />);
+
+    assert.equal(wrapper.text(), "hello");
+  });
+  it("should use fluent id on automatic span", () => {
+    const wrapper = shallow(<FluentOrText message={{ id: "fluent" }} />);
+
+    assert.ok(wrapper.find("span[data-l10n-id='fluent']"));
+  });
+  it("should also allow string_id", () => {
+    const wrapper = shallow(<FluentOrText message={{ string_id: "fluent" }} />);
+
+    assert.ok(wrapper.find("span[data-l10n-id='fluent']"));
+  });
+  it("should use fluent id on child", () => {
+    const wrapper = shallow(
+      <FluentOrText message={{ id: "fluent" }}>
+        <p />
+      </FluentOrText>
+    );
+
+    assert.ok(wrapper.find("p[data-l10n-id='fluent']"));
+  });
+  it("should set args for fluent", () => {
+    const wrapper = shallow(<FluentOrText message={{ args: { num: 5 } }} />);
+
+    assert.ok(wrapper.find("span[data-l10n-args='{num: 5}']"));
+  });
+  it("should also allow values", () => {
+    const wrapper = shallow(<FluentOrText message={{ values: { num: 5 } }} />);
+
+    assert.ok(wrapper.find("span[data-l10n-args='{num: 5}']"));
+  });
+  it("should preserve original children with fluent", () => {
+    const wrapper = shallow(
+      <FluentOrText message={{ id: "fluent" }}>
+        <p>
+          <b data-l10n-name="bold" />
+        </p>
+      </FluentOrText>
+    );
+
+    assert.ok(wrapper.find("b[data-l10n-name='bold']"));
+  });
+  it("should only allow a single child", () => {
+    assert.throws(() =>
+      shallow(
+        <FluentOrText>
+          <p />
+          <p />
+        </FluentOrText>
+      )
+    );
+  });
+});