Bug 1552596 - Revert dynamic line clamping in favor of performant static line clamping (#5044)

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

@@ -1,5 +1,4 @@
 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";
@@ -41,11 +40,9 @@ 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"
-              data-total-lines="7"
-              ref={clampTotalLines}>
-              <p className="source clamp" data-clamp="1">{this.props.source}</p>
-              <header className="title clamp" data-clamp="4">{this.props.title}</header>
+            <div className="info-wrap">
+              <p className="source clamp">{this.props.source}</p>
+              <header className="title clamp">{this.props.title}</header>
               {this.props.excerpt && <p className="excerpt clamp">{this.props.excerpt}</p>}
             </div>
             {this.props.context && (

content-src/components/DiscoveryStreamComponents/DSCard/_DSCard.scss

@@ -101,6 +101,7 @@ $excerpt-line-height: 20;
     }
 
     .source {
+      -webkit-line-clamp: 1;
       margin-bottom: 2px;
     }
 

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

@@ -1,6 +1,5 @@
 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";
@@ -77,15 +76,13 @@ 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"
-                data-total-lines="7"
-                ref={clampTotalLines}>
+              <div className="header-and-excerpt">
                 {heroRec.context ? (
                   <p className="context">{heroRec.context}</p>
                 ) : (
-                  <p className="source clamp" data-clamp="1">{heroRec.domain}</p>
+                  <p className="source clamp">{heroRec.domain}</p>
                 )}
-                <header className="clamp" data-clamp="4">{heroRec.title}</header>
+                <header className="clamp">{heroRec.title}</header>
                 <p className="excerpt clamp">{heroRec.excerpt}</p>
               </div>
             </div>

content-src/components/DiscoveryStreamComponents/Hero/_Hero.scss

@@ -147,6 +147,7 @@ $card-header-in-hero-line-height: 20;
 
         font-size: 13px;
         color: $grey-50;
+        -webkit-line-clamp: 1;
         margin-bottom: 0;
       }
     }

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

@@ -1,5 +1,4 @@
 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";
@@ -44,15 +43,14 @@ export class ListItem extends React.PureComponent {
           onLinkClick={!this.props.placeholder ? this.onLinkClick : undefined}
           url={this.props.url}>
           <div className="ds-list-item-text">
-            <div data-total-lines="4"
-              ref={clampTotalLines}>
+            <div>
               <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 && (
                 <span>
-                  <span className="ds-list-item-context">{this.props.context}</span>
+                  <span className="ds-list-item-context clamp">{this.props.context}</span>
                   <br />
                 </span>
               )}

content-src/components/DiscoveryStreamComponents/List/_List.scss

@@ -228,7 +228,6 @@ $item-line-height: 20;
 
     color: $grey-50;
     font-size: 13px;
-    -webkit-line-clamp: 1;
   }
 
   .ds-list-item-title {

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

@@ -1,32 +0,0 @@
-/**
- * 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/_mixins.scss

@@ -9,9 +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
+// Bug 1550624 to clean up / remove this mixin to avoid duplicate styles
 @mixin limit-visibile-lines($line-count, $line-height, $font-size) {
   font-size: $font-size * 1px;
+  -webkit-line-clamp: $line-count;
   line-height: $line-height * 1px;
 }
 

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

@@ -1,154 +0,0 @@
-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);
-  });
-});