Bug 1551359 - Fill cards with longer excerpts for shorter titles (#5025)

content-src/components/DiscoveryStreamComponents/DSCard/DSCard.jsx

@@ -1,4 +1,5 @@
 import {actionCreators as ac} from "common/Actions.jsm";
+import {clampTotalLines} from "content-src/lib/clamp-total-lines";
 import {DSImage} from "../DSImage/DSImage.jsx";
 import {DSLinkMenu} from "../DSLinkMenu/DSLinkMenu";
 import {ImpressionStats} from "../../DiscoveryStreamImpressionStats/ImpressionStats";
@@ -40,10 +41,12 @@ export class DSCard extends React.PureComponent {
             <DSImage extraClassNames="img" source={this.props.image_src} rawSource={this.props.raw_image_src} />
           </div>
           <div className="meta">
-            <div className="info-wrap">
+            <div className="info-wrap"
+              data-total-lines="6"
+              ref={clampTotalLines}>
               <p className="source">{this.props.source}</p>
-              <header className="title">{this.props.title}</header>
-              {this.props.excerpt && <p className="excerpt">{this.props.excerpt}</p>}
+              <header className="title clamp" data-clamp="4">{this.props.title}</header>
+              {this.props.excerpt && <p className="excerpt clamp">{this.props.excerpt}</p>}
             </div>
             {this.props.context && (
               <p className="context">{this.props.context}</p>

content-src/components/DiscoveryStreamComponents/Hero/Hero.jsx

@@ -1,5 +1,6 @@
 import {DSCard, PlaceholderDSCard} from "../DSCard/DSCard.jsx";
 import {actionCreators as ac} from "common/Actions.jsm";
+import {clampTotalLines} from "content-src/lib/clamp-total-lines";
 import {DSEmptyState} from "../DSEmptyState/DSEmptyState.jsx";
 import {DSImage} from "../DSImage/DSImage.jsx";
 import {DSLinkMenu} from "../DSLinkMenu/DSLinkMenu";
@@ -76,14 +77,16 @@ export class Hero extends React.PureComponent {
               <DSImage extraClassNames="img" source={heroRec.image_src} rawSource={heroRec.raw_image_src} />
             </div>
             <div className="meta">
-              <div className="header-and-excerpt">
+              <div className="header-and-excerpt"
+                data-total-lines="6"
+                ref={clampTotalLines}>
                 {heroRec.context ? (
                   <p className="context">{heroRec.context}</p>
                 ) : (
                   <p className="source">{heroRec.domain}</p>
                 )}
-                <header>{heroRec.title}</header>
-                <p className="excerpt">{heroRec.excerpt}</p>
+                <header className="clamp" data-clamp="4">{heroRec.title}</header>
+                <p className="excerpt clamp">{heroRec.excerpt}</p>
               </div>
             </div>
             <ImpressionStats

content-src/components/DiscoveryStreamComponents/List/List.jsx

@@ -1,4 +1,5 @@
 import {actionCreators as ac} from "common/Actions.jsm";
+import {clampTotalLines} from "content-src/lib/clamp-total-lines";
 import {connect} from "react-redux";
 import {DSEmptyState} from "../DSEmptyState/DSEmptyState.jsx";
 import {DSImage} from "../DSImage/DSImage.jsx";
@@ -43,9 +44,10 @@ export class ListItem extends React.PureComponent {
           onLinkClick={!this.props.placeholder ? this.onLinkClick : undefined}
           url={this.props.url}>
           <div className="ds-list-item-text">
-            <div>
-              <div className="ds-list-item-title">{this.props.title}</div>
-              {this.props.excerpt && <div className="ds-list-item-excerpt">{this.props.excerpt}</div>}
+            <div data-total-lines="4"
+              ref={clampTotalLines}>
+              <div className="ds-list-item-title clamp">{this.props.title}</div>
+              {this.props.excerpt && <div className="ds-list-item-excerpt clamp">{this.props.excerpt}</div>}
             </div>
             <p>
               {this.props.context && (

content-src/lib/clamp-total-lines.js

@@ -0,0 +1,32 @@
+/**
+ * Adjusts line-clamps of a node's children to fill a desired number of lines.
+ *
+ * This is a React callback ref that should be set on a parent node that also
+ * has a data-total-lines attribute. Children with "clamp" class name are
+ * clamped to allow as many lines to earlier children while making sure every
+ * child gets at least one line. Each child can be explicitly clamped to a max
+ * lines with a data-clamp attribute.
+ */
+export function clampTotalLines(parentNode) {
+  // Nothing to do if the node is removed or didn't configure how many lines
+  if (!parentNode || !parentNode.dataset.totalLines) {
+    return;
+  }
+
+  // Only handle clamp-able children that are displayed (not hidden)
+  const toClamp = Array.from(parentNode.querySelectorAll(".clamp"))
+    .filter(child => child.scrollHeight);
+
+  // Start with total allowed lines while reserving 1 line for each child
+  let maxLines = parentNode.dataset.totalLines - toClamp.length + 1;
+  toClamp.forEach(child => {
+    // Clamp to the remaining allowed, explicit limit or the natural line count
+    const lines = Math.min(maxLines,
+      child.dataset.clamp || Infinity,
+      child.scrollHeight / parseInt(global.getComputedStyle(child).lineHeight, 10));
+    child.style.webkitLineClamp = `${lines}`;
+
+    // Update the remaining line allowance less the already reserved 1 line
+    maxLines -= lines - 1;
+  });
+}

content-src/styles/_activity-stream.scss

@@ -132,6 +132,14 @@ input {
   }
 }
 
+// These styles are needed for -webkit-line-clamp to work correctly, so reuse
+// this class name while separately setting a clamp value via CSS or JS.
+.clamp {
+  -webkit-box-orient: vertical;
+  display: -webkit-box;
+  overflow: hidden;
+}
+
 // Components
 @import '../components/Base/Base';
 @import '../components/ErrorBoundary/ErrorBoundary';

content-src/styles/_mixins.scss

@@ -9,14 +9,10 @@
 }
 
 // Note: lineHeight and fontSize should be unitless but can be derived from pixel values
+// Bug 1550624 to clean up / remove this mixin that no longer limits lines
 @mixin limit-visibile-lines($line-count, $line-height, $font-size) {
-  -webkit-box-orient: vertical;
-  display: -webkit-box;
   font-size: $font-size * 1px;
-  -webkit-line-clamp: $line-count;
   line-height: $line-height * 1px;
-  max-height: 1em * $line-count * $line-height / $font-size;
-  overflow: hidden;
 }
 
 @mixin dark-theme-only {

test/unit/content-src/lib/clamp-total-lines.test.js

@@ -0,0 +1,154 @@
+import {clampTotalLines} from "content-src/lib/clamp-total-lines";
+import {GlobalOverrider} from "test/unit/utils";
+
+const HEIGHT = 20;
+
+describe("clampTotalLines", () => {
+  let globals;
+  let sandbox;
+  let children;
+
+  const node = () => document.createElement("div");
+  function child(lines, clamp) {
+    const ret = node();
+    ret.classList.add("clamp");
+    sandbox.stub(ret, "scrollHeight").get(() => lines * HEIGHT);
+    if (clamp) {
+      ret.dataset.clamp = clamp;
+    }
+    return ret;
+  }
+  function test(totalLines) {
+    const parentNode = node();
+    parentNode.setAttribute("data-total-lines", totalLines);
+
+    // Convert children line sizes into clamp nodes with appropriate height
+    children = children.map(childLines => parentNode.appendChild(
+      typeof childLines === "number" ? child(childLines) : childLines));
+
+    clampTotalLines(parentNode);
+  }
+  function check(index, lines) {
+    assert.propertyVal(children[index].style, "webkitLineClamp", `${lines}`);
+  }
+
+  beforeEach(() => {
+    globals = new GlobalOverrider();
+    ({sandbox} = globals);
+    children = [];
+    globals.set("getComputedStyle", () => ({lineHeight: `${HEIGHT}px`}));
+  });
+  afterEach(() => {
+    globals.restore();
+  });
+
+  it("should do nothing with nothing", () => {
+    clampTotalLines();
+  });
+  it("should clamp single long child to total", () => {
+    children = [10];
+
+    test(6);
+
+    check(0, 6);
+  });
+  it("should clamp single short child to its height", () => {
+    children = [2];
+
+    test(6);
+
+    check(0, 2);
+  });
+  it("should clamp long children preferring first", () => {
+    children = [10, 10];
+
+    test(6);
+
+    check(0, 5);
+    check(1, 1);
+  });
+  it("should clamp short children to their heights", () => {
+    children = [2, 2];
+
+    test(6);
+
+    check(0, 2);
+    check(1, 2);
+  });
+  it("should give remainder to last child", () => {
+    children = [4, 4];
+
+    test(6);
+
+    check(0, 4);
+    check(1, 2);
+  });
+  it("should handle smaller totals", () => {
+    children = [3, 3];
+
+    test(4);
+
+    check(0, 3);
+    check(1, 1);
+  });
+  it("should allow explicit child clamp", () => {
+    children = [child(3, 2), 3];
+
+    test(4);
+
+    check(0, 2);
+    check(1, 2);
+  });
+  it("should skip non-clamp children", () => {
+    children = [node(), 3, node(), 3];
+
+    test(4);
+
+    check(1, 3);
+    check(3, 1);
+  });
+  it("should skip no-height children", () => {
+    children = [0, 3, 0, 3];
+
+    test(4);
+
+    check(1, 3);
+    check(3, 1);
+  });
+  it("should handle larger totals", () => {
+    children = [4, 4];
+
+    test(8);
+
+    check(0, 4);
+    check(1, 4);
+  });
+  it("should handle even larger totals", () => {
+    children = [4, 4];
+
+    test(10);
+
+    check(0, 4);
+    check(1, 4);
+  });
+  it("should clamp many children preferring earlier", () => {
+    children = [2, 2, 2, 2];
+
+    test(6);
+
+    check(0, 2);
+    check(1, 2);
+    check(2, 1);
+    check(3, 1);
+  });
+  it("should give lines to children that need them", () => {
+    children = [1, 2, 3, 4];
+
+    test(8);
+
+    check(0, 1);
+    check(1, 2);
+    check(2, 3);
+    check(3, 2);
+  });
+});