Do not show edited timestamp details if it duplicates created timestamp

Only show the relative time-since string for the annotation edited date
if it is different from the relative time-since string for the created
date. If they are the same, just show "edited" without further detail.

Fixes https://github.com/hypothesis/product-backlog/issues/1156

src/sidebar/components/annotation-header.js

@@ -4,9 +4,9 @@ import propTypes from 'prop-types';
 
 import useStore from '../store/use-store';
 import {
-  hasBeenEdited,
   isHighlight,
   isReply,
+  hasBeenEdited,
 } from '../util/annotation-metadata';
 import { isPrivate } from '../util/permissions';
 
@@ -15,7 +15,7 @@ import AnnotationShareInfo from './annotation-share-info';
 import AnnotationUser from './annotation-user';
 import Button from './button';
 import SvgIcon from '../../shared/components/svg-icon';
-import Timestamp from './timestamp';
+import AnnotationTimestamps from './annotation-timestamps';
 
 /**
  * @typedef {import("../../types/api").Annotation} Annotation
@@ -50,9 +50,8 @@ export default function AnnotationHeader({
   const setExpanded = useStore(store => store.setExpanded);
 
   const annotationIsPrivate = isPrivate(annotation.permissions);
-  const annotationLink = annotation.links ? annotation.links.html : '';
 
-  const showTimestamp = !isEditing && annotation.created;
+  const showTimestamps = !isEditing && annotation.created;
   const showEditedTimestamp = useMemo(() => {
     return hasBeenEdited(annotation) && !isCollapsedReply;
   }, [annotation, isCollapsedReply]);
@@ -87,25 +86,12 @@ export default function AnnotationHeader({
           />
         )}
 
-        {showTimestamp && (
-          <div className="annotation-header__timestamp">
-            {showEditedTimestamp && (
-              <span className="annotation-header__timestamp-edited">
-                (edited{' '}
-                <Timestamp
-                  className="annotation-header__timestamp-edited-link"
-                  timestamp={annotation.updated}
-                />
-                ){' '}
-              </span>
-            )}
-            <span className="annotation-header__timestamp-created">
-              <Timestamp
-                className="annotation-header__timestamp-created-link u-color-text--muted"
-                href={annotationLink}
-                timestamp={annotation.created}
+        {showTimestamps && (
+          <div className="annotation-header__timestamps">
+            <AnnotationTimestamps
+              annotation={annotation}
+              withEditedTimestamp={showEditedTimestamp}
             />
-            </span>
           </div>
         )}
       </div>

src/sidebar/components/annotation-timestamps.js

+import { createElement } from 'preact';
+import { useEffect, useMemo, useState } from 'preact/hooks';
+import propTypes from 'prop-types';
+
+import { format as formatDate } from '../util/date';
+import { decayingInterval, toFuzzyString } from '../util/time';
+
+/**
+ * @typedef {import("../../types/api").Annotation} Annotation
+ */
+
+/**
+ * @typedef AnnotationTimestampsProps
+ * @prop {Annotation} annotation
+ * @prop {boolean} [withEditedTimestamp] - Should a timestamp for when this
+ *   annotation was last edited be rendered?
+ */
+
+/**
+ * Render textual timestamp information for an annotation. This includes
+ * a relative date string (e.g. "5 hours ago") for the annotation's creation,
+ * and, if `withEditedTimestamp` is `true`, a relative date string for when it
+ * was last edited. If the `annotation` has an HTML link, the created-date
+ * timestamp will be linked to that URL (the single-annotation view
+ * for this annotation).
+ *
+ * @param {AnnotationTimestampsProps} props
+ */
+export default function AnnotationTimestamps({
+  annotation,
+  withEditedTimestamp,
+}) {
+  // "Current" time, used when calculating the relative age of `timestamp`.
+  const [now, setNow] = useState(() => new Date());
+  const createdDate = useMemo(() => new Date(annotation.created), [
+    annotation.created,
+  ]);
+  const updatedDate = useMemo(
+    () => withEditedTimestamp && new Date(annotation.updated),
+    [annotation.updated, withEditedTimestamp]
+  );
+
+  const created = useMemo(() => {
+    return {
+      absolute: formatDate(createdDate),
+      relative: toFuzzyString(createdDate, now),
+    };
+  }, [createdDate, now]);
+
+  const updated = useMemo(() => {
+    if (!updatedDate) {
+      return {};
+    }
+    return {
+      absolute: formatDate(updatedDate),
+      relative: toFuzzyString(updatedDate, now),
+    };
+  }, [updatedDate, now]);
+
+  // Refresh relative timestamp, at a frequency appropriate for the age.
+  useEffect(() => {
+    // Determine which of the two Dates to use for the `decayingInterval`
+    // It should be the latest relevant date, as the interval will be
+    // shorter the closer the date is to "now"
+    const laterDate = updatedDate ? annotation.updated : annotation.created;
+    const cancelRefresh = decayingInterval(laterDate, () => setNow(new Date()));
+    return cancelRefresh;
+  }, [annotation, createdDate, updatedDate, now]);
+
+  // Do not show the relative timestamp for the edited date if it is the same
+  // as the relative timestamp for the created date
+  const editedString =
+    updated && updated.relative !== created.relative
+      ? `edited ${updated.relative}`
+      : 'edited';
+
+  const annotationUrl = annotation.links?.html || '';
+
+  return (
+    <div className="annotation-timestamps">
+      {withEditedTimestamp && (
+        <span
+          className="annotation-timestamps__edited"
+          title={updated.absolute}
+        >
+          ({editedString}){' '}
+        </span>
+      )}
+      {annotationUrl ? (
+        <a
+          className="annotation-timestamps__created"
+          target="_blank"
+          rel="noopener noreferrer"
+          title={created.absolute}
+          href={annotationUrl}
+        >
+          {created.relative}
+        </a>
+      ) : (
+        <span
+          className="annotation-timestamps__created"
+          title={created.absolute}
+        >
+          {created.relative}
+        </span>
+      )}
+    </div>
+  );
+}
+
+AnnotationTimestamps.propTypes = {
+  annotation: propTypes.object.isRequired,
+  withEditedTimestamp: propTypes.bool,
+};

src/sidebar/components/test/annotation-header-test.js

@@ -164,39 +164,42 @@ describe('AnnotationHeader', () => {
   });
 
   describe('timestamps', () => {
-    it('should render timestamp container element if annotation has a `created` value', () => {
+    it('should not render timestamps if annotation is missing `created` date', () => {
+      const annotation = fixtures.defaultAnnotation();
+      delete annotation.created;
+      const wrapper = createAnnotationHeader({ annotation });
+
+      const timestamp = wrapper.find('AnnotationTimestamps');
+
+      assert.isFalse(timestamp.exists());
+    });
+
+    it('should render timestamps  if annotation has a `created` value', () => {
       const wrapper = createAnnotationHeader();
-      const timestamp = wrapper.find('.annotation-header__timestamp');
+      const timestamp = wrapper.find('AnnotationTimestamps');
 
       assert.isTrue(timestamp.exists());
     });
 
-    it('should render edited timestamp if annotation has been edited', () => {
+    it('should render `updated` timestamp if annotation has an `updated` value', () => {
       const annotation = fixtures.defaultAnnotation();
       fakeHasBeenEdited.returns(true);
 
       const wrapper = createAnnotationHeader({
         annotation: annotation,
       });
-      const timestamp = wrapper
-        .find('Timestamp')
-        .filter('.annotation-header__timestamp-edited-link');
-
-      assert.isTrue(timestamp.exists());
+      const timestamp = wrapper.find('AnnotationTimestamps');
+      assert.equal(timestamp.props().withEditedTimestamp, true);
     });
 
     it('should not render edited timestamp if annotation has not been edited', () => {
       // Default annotation's created value is same as updated; as if the annotation
       // has not been edited before
       fakeHasBeenEdited.returns(false);
-      const wrapper = createAnnotationHeader({
-        annotation: fixtures.newAnnotation(),
-      });
-      const timestamp = wrapper
-        .find('Timestamp')
-        .filter('.annotation-header__timestamp-edited-link');
+      const wrapper = createAnnotationHeader();
 
-      assert.isFalse(timestamp.exists());
+      const timestamp = wrapper.find('AnnotationTimestamps');
+      assert.equal(timestamp.props().withEditedTimestamp, false);
     });
 
     it('should not render edited timestamp if annotation is collapsed reply', () => {
@@ -210,11 +213,8 @@ describe('AnnotationHeader', () => {
         threadIsCollapsed: true,
       });
 
-      const timestamp = wrapper
-        .find('Timestamp')
-        .filter('.annotation-header__timestamp-edited-link');
-
-      assert.isFalse(timestamp.exists());
+      const timestamp = wrapper.find('AnnotationTimestamps');
+      assert.equal(timestamp.props().withEditedTimestamp, false);
     });
   });
 
@@ -277,7 +277,7 @@ describe('AnnotationHeader', () => {
         isEditing: true,
       });
 
-      const timestamp = wrapper.find('Timestamp');
+      const timestamp = wrapper.find('AnnotationTimestamps');
 
       assert.isFalse(timestamp.exists());
     });

src/sidebar/components/test/annotation-timestamps-test.js

+import { mount } from 'enzyme';
+import { createElement } from 'preact';
+import * as fixtures from '../../test/annotation-fixtures';
+import { act } from 'preact/test-utils';
+
+import AnnotationTimestamps from '../annotation-timestamps';
+import { $imports } from '../annotation-timestamps';
+
+import { checkAccessibility } from '../../../test-util/accessibility';
+
+describe('AnnotationTimestamps', () => {
+  let clock;
+  let fakeFormatDate;
+  let fakeTime;
+  let fakeToFuzzyString;
+
+  const createComponent = props =>
+    mount(
+      <AnnotationTimestamps
+        annotation={fixtures.defaultAnnotation()}
+        withEditedTimestamp={false}
+        {...props}
+      />
+    );
+
+  beforeEach(() => {
+    clock = sinon.useFakeTimers();
+    fakeToFuzzyString = sinon.stub().returns('fuzzy string');
+    fakeFormatDate = sinon.stub().returns('absolute date');
+
+    fakeTime = {
+      toFuzzyString: fakeToFuzzyString,
+      decayingInterval: sinon.stub(),
+    };
+
+    $imports.$mock({
+      '../util/date': { format: fakeFormatDate },
+      '../util/time': fakeTime,
+    });
+  });
+
+  afterEach(() => {
+    clock.restore();
+    $imports.$restore();
+  });
+
+  it('renders a linked created timestamp if annotation has a link', () => {
+    const annotation = fixtures.defaultAnnotation();
+    annotation.links = { html: 'http://www.example.com' };
+
+    const wrapper = createComponent({ annotation });
+
+    const link = wrapper.find('a.annotation-timestamps__created');
+    assert.equal(link.prop('href'), 'http://www.example.com');
+    assert.equal(link.prop('title'), 'absolute date');
+    assert.equal(link.text(), 'fuzzy string');
+  });
+
+  it('renders an unlinked created timestamp if annotation does not have a link', () => {
+    const wrapper = createComponent();
+
+    const link = wrapper.find('a.annotation-timestamps__created');
+    const span = wrapper.find('span.annotation-timestamps__created');
+    assert.isFalse(link.exists());
+    assert.isTrue(span.exists());
+    assert.equal(span.text(), 'fuzzy string');
+  });
+
+  it('renders edited timestamp if `withEditedTimestamp` is true', () => {
+    fakeToFuzzyString.onCall(1).returns('another fuzzy string');
+
+    const wrapper = createComponent({ withEditedTimestamp: true });
+
+    const editedTimestamp = wrapper.find('.annotation-timestamps__edited');
+    assert.isTrue(editedTimestamp.exists());
+    assert.include(editedTimestamp.text(), '(edited another fuzzy string)');
+  });
+
+  it('does not render edited relative date if equivalent to created relative date', () => {
+    fakeToFuzzyString.returns('equivalent fuzzy strings');
+
+    const wrapper = createComponent({ withEditedTimestamp: true });
+
+    const editedTimestamp = wrapper.find('.annotation-timestamps__edited');
+    assert.isTrue(editedTimestamp.exists());
+    assert.include(editedTimestamp.text(), '(edited)');
+  });
+
+  it('is updated after time passes', () => {
+    fakeTime.decayingInterval.callsFake((date, callback) =>
+      setTimeout(callback, 10)
+    );
+    const wrapper = createComponent();
+    fakeTime.toFuzzyString.returns('60 jiffies');
+
+    act(() => {
+      clock.tick(1000);
+    });
+    wrapper.update();
+
+    assert.equal(wrapper.text(), '60 jiffies');
+  });
+
+  it(
+    'should pass a11y checks',
+    checkAccessibility({
+      content: () => {
+        // Fake timers break axe-core.
+        clock.restore();
+
+        return createComponent();
+      },
+    })
+  );
+});

src/sidebar/components/test/timestamp-test.js

-import { mount } from 'enzyme';
-import { createElement } from 'preact';
-import { act } from 'preact/test-utils';
-
-import Timestamp from '../timestamp';
-import { $imports } from '../timestamp';
-
-import { checkAccessibility } from '../../../test-util/accessibility';
-
-describe('Timestamp', () => {
-  let clock;
-  let fakeTime;
-
-  beforeEach(() => {
-    clock = sinon.useFakeTimers();
-    fakeTime = {
-      toFuzzyString: sinon.stub().returns('a while ago'),
-      decayingInterval: sinon.stub(),
-    };
-
-    $imports.$mock({
-      '../util/time': fakeTime,
-    });
-  });
-
-  afterEach(() => {
-    clock.restore();
-    $imports.$restore();
-  });
-
-  const createTimestamp = props => mount(<Timestamp {...props} />);
-
-  it('displays a link if an "href" is provided', () => {
-    const wrapper = createTimestamp({
-      timestamp: '2016-06-10',
-      href: 'https://example.com',
-    });
-    const link = wrapper.find('a');
-    assert.equal(link.length, 1);
-    assert.equal(link.prop('href'), 'https://example.com');
-  });
-
-  it('displays static text if no "href" is provided', () => {
-    const wrapper = createTimestamp({ timestamp: '2016-06-10' });
-    assert.equal(wrapper.find('a').length, 0);
-    assert.equal(wrapper.find('span').length, 1);
-  });
-
-  describe('timestamp label', () => {
-    it('displays a relative time string', () => {
-      const wrapper = createTimestamp({
-        timestamp: '2016-06-10T10:04:04.939Z',
-      });
-      assert.equal(wrapper.text(), 'a while ago');
-    });
-
-    it('is updated when the timestamp changes', () => {
-      const wrapper = createTimestamp({
-        timestamp: '1776-07-04T10:04:04.939Z',
-      });
-
-      fakeTime.toFuzzyString.returns('four score and seven years ago');
-      wrapper.setProps({ timestamp: '1863-11-19T12:00:00.939Z' });
-
-      assert.equal(wrapper.text(), 'four score and seven years ago');
-    });
-
-    it('is updated after time passes', () => {
-      fakeTime.decayingInterval.callsFake((date, callback) =>
-        setTimeout(callback, 10)
-      );
-      const wrapper = createTimestamp({
-        timestamp: '2016-06-10T10:04:04.939Z',
-      });
-      fakeTime.toFuzzyString.returns('60 jiffies');
-
-      act(() => {
-        clock.tick(1000);
-      });
-      wrapper.update();
-
-      assert.equal(wrapper.text(), '60 jiffies');
-    });
-
-    it('is no longer updated after the component is destroyed', () => {
-      const wrapper = createTimestamp({
-        timestamp: '2016-06-10T10:04:04.939Z',
-      });
-
-      wrapper.unmount();
-
-      assert.called(fakeTime.decayingInterval);
-    });
-  });
-
-  describe('timestamp tooltip', () => {
-    [
-      {
-        variant: 'without link',
-        href: null,
-      },
-      {
-        variant: 'with link',
-        href: 'https://annotate.com/a/1234',
-      },
-    ].forEach(({ variant, href }) => {
-      it(`displays an absolute timestamp (${variant})`, () => {
-        const date = new Date('2016-06-10T10:04:04.939Z');
-        const format = date => `formatted:${date}`;
-        $imports.$mock({
-          '../util/date': {
-            format,
-          },
-        });
-
-        const wrapper = createTimestamp({
-          timestamp: date.toISOString(),
-          href,
-        });
-
-        assert.match(
-          wrapper
-            .findWhere(n => n.type() === 'a' || n.type() === 'span')
-            .prop('title'),
-          format(date)
-        );
-      });
-    });
-  });
-
-  it(
-    'should pass a11y checks',
-    checkAccessibility({
-      content: () => {
-        // Fake timers break axe-core.
-        clock.restore();
-
-        return createTimestamp({
-          timestamp: '2016-06-10T10:04:04.939Z',
-          href: 'https://annotate.com/a/1234',
-        });
-      },
-    })
-  );
-});

src/sidebar/components/timestamp.js

-import { createElement } from 'preact';
-import { useEffect, useMemo, useState } from 'preact/hooks';
-import propTypes from 'prop-types';
-
-import { format as formatDate } from '../util/date';
-import { decayingInterval, toFuzzyString } from '../util/time';
-
-/**
- * @typedef TimestampProps
- * @prop {string} [className] - Custom class name for the anchor/span element
- * @prop {string} [href] - Link destination
- * @prop {string} timestamp - The timestamp as an ISO 8601 date string
- */
-
-/**
- * Display a relative timestamp (eg. '6 minutes ago') as static text or a link.
- *
- * @param {TimestampProps} props
- *
- * The timestamp automatically refreshes at an appropriate frequency.
- */
-export default function Timestamp({ className, href, timestamp }) {
-  // "Current" time, used when calculating the relative age of `timestamp`.
-  const [now, setNow] = useState(new Date());
-
-  // Fuzzy, relative timestamp (eg. '6 days ago')
-  const relativeTimestamp = useMemo(
-    () => toFuzzyString(timestamp ? new Date(timestamp) : null, now),
-    [timestamp, now]
-  );
-
-  // Absolute timestamp (eg. 'Tue 22nd Dec 2015, 16:00')
-  const absoluteTimestamp = useMemo(() => formatDate(new Date(timestamp)), [
-    timestamp,
-  ]);
-
-  // Refresh relative timestamp, at a frequency appropriate for the age.
-  useEffect(() => {
-    const cancelRefresh = decayingInterval(timestamp, () => setNow(new Date()));
-    return cancelRefresh;
-  }, [timestamp]);
-
-  return href ? (
-    <a
-      className={className}
-      target="_blank"
-      rel="noopener noreferrer"
-      title={absoluteTimestamp}
-      href={href}
-    >
-      {relativeTimestamp}
-    </a>
-  ) : (
-    <span className={className} title={absoluteTimestamp}>
-      {relativeTimestamp}
-    </span>
-  );
-}
-
-Timestamp.propTypes = {
-  className: propTypes.string,
-  href: propTypes.string,
-  timestamp: propTypes.string.isRequired,
-};

src/styles/sidebar/components/annotation-header.scss

@@ -23,22 +23,10 @@
   }
 
   // Timestamps are right aligned in a flex row
-  &__timestamp {
+  &__timestamps {
     margin-left: auto;
   }
 
-  &__timestamp-edited {
-    @include utils.font--small;
-    font-style: italic;
-    color: var.$color-text--light;
-  }
-
-  &__timestamp-created-link {
-    &:hover {
-      text-decoration: underline;
-    }
-  }
-
   &__highlight-icon {
     @include utils.icon--small;
     color: var.$color-text--light;

src/styles/sidebar/components/annotation-timestamps.scss

+@use "../../variables" as var;
+@use "../../mixins/utils";
+
+.annotation-timestamps {
+  &__edited {
+    @include utils.font--small;
+    font-style: italic;
+    color: var.$color-text--light;
+  }
+
+  &__created {
+    color: var.$color-text--light;
+    &:hover {
+      text-decoration: underline;
+    }
+  }
+}

src/styles/sidebar/sidebar.scss

@@ -21,6 +21,7 @@
 @use './components/annotation-quote';
 @use './components/annotation-share-control';
 @use './components/annotation-share-info';
+@use './components/annotation-timestamps';
 @use './components/annotation-user';
 @use './components/autocomplete-list';
 @use './components/button';