Commit bc58c21c authored by Lyza Danger Gardner's avatar Lyza Danger Gardner Committed by Lyza Gardner

Simplify `AnnotationTimestamps`

Make `AnnotationTimestamps` align with changes to `ShowDocumentInfo`;
it is now a dumb component. Centralize logic in `AnnotationHeader`.
parent 0aa62e16
...@@ -48,6 +48,11 @@ export default function AnnotationHeader({ ...@@ -48,6 +48,11 @@ export default function AnnotationHeader({
const annotationIsPrivate = isPrivate(annotation.permissions); const annotationIsPrivate = isPrivate(annotation.permissions);
// Link (URL) to single-annotation view for this annotation, if it has
// been provided by the service. Note: this property is not currently
// present on third-party annotations.
const annotationUrl = annotation.links?.html || '';
const showTimestamps = !isEditing && annotation.created; const showTimestamps = !isEditing && annotation.created;
const showEditedTimestamp = useMemo(() => { const showEditedTimestamp = useMemo(() => {
return hasBeenEdited(annotation) && !isCollapsedReply; return hasBeenEdited(annotation) && !isCollapsedReply;
...@@ -58,10 +63,22 @@ export default function AnnotationHeader({ ...@@ -58,10 +63,22 @@ export default function AnnotationHeader({
const showReplyButton = replyCount > 0 && isCollapsedReply; const showReplyButton = replyCount > 0 && isCollapsedReply;
const showExtendedInfo = !isReply(annotation); const showExtendedInfo = !isReply(annotation);
const annotationUrl = annotation.links?.html || ''; // Pull together some document metadata related to this annotation
const documentInfo = domainAndTitle(annotation); const documentInfo = domainAndTitle(annotation);
// There are some cases at present in which linking directly to an
// annotation's document is not immediately feasible—e.g in an LMS context
// where the original document might not be available outside of an
// assignment (e.g. Canvas files), and/or wouldn't be able to present
// any associated annotations.
// For the present, disable links to annotation documents for all third-party
// annotations until we have a more nuanced way of making linking determinations.
// The absence of a link to a single-annotation view is a signal that this
// is a third-party annotation.
// Also, of course, verify that there is a URL to the document (titleLink)
const documentLink = const documentLink =
annotationUrl && documentInfo.titleLink ? documentInfo.titleLink : ''; annotationUrl && documentInfo.titleLink ? documentInfo.titleLink : '';
// Show document information on non-sidebar routes, assuming there is a title
// to show, at the least
const showDocumentInfo = const showDocumentInfo =
store.route() !== 'sidebar' && documentInfo.titleText; store.route() !== 'sidebar' && documentInfo.titleText;
...@@ -93,7 +110,9 @@ export default function AnnotationHeader({ ...@@ -93,7 +110,9 @@ export default function AnnotationHeader({
{showTimestamps && ( {showTimestamps && (
<div className="AnnotationHeader__timestamps"> <div className="AnnotationHeader__timestamps">
<AnnotationTimestamps <AnnotationTimestamps
annotation={annotation} annotationCreated={annotation.created}
annotationUpdated={annotation.updated}
annotationUrl={annotationUrl}
withEditedTimestamp={showEditedTimestamp} withEditedTimestamp={showEditedTimestamp}
/> />
</div> </div>
......
...@@ -10,7 +10,9 @@ import { decayingInterval, toFuzzyString } from '../../util/time'; ...@@ -10,7 +10,9 @@ import { decayingInterval, toFuzzyString } from '../../util/time';
/** /**
* @typedef AnnotationTimestampsProps * @typedef AnnotationTimestampsProps
* @prop {Annotation} annotation * @prop {string} annotationCreated
* @prop {string} annotationUpdated
* @prop {string} [annotationUrl]
* @prop {boolean} [withEditedTimestamp] - Should a timestamp for when this * @prop {boolean} [withEditedTimestamp] - Should a timestamp for when this
* annotation was last edited be rendered? * annotation was last edited be rendered?
*/ */
...@@ -26,17 +28,19 @@ import { decayingInterval, toFuzzyString } from '../../util/time'; ...@@ -26,17 +28,19 @@ import { decayingInterval, toFuzzyString } from '../../util/time';
* @param {AnnotationTimestampsProps} props * @param {AnnotationTimestampsProps} props
*/ */
export default function AnnotationTimestamps({ export default function AnnotationTimestamps({
annotation, annotationCreated,
annotationUpdated,
annotationUrl,
withEditedTimestamp, withEditedTimestamp,
}) { }) {
// "Current" time, used when calculating the relative age of `timestamp`. // "Current" time, used when calculating the relative age of `timestamp`.
const [now, setNow] = useState(() => new Date()); const [now, setNow] = useState(() => new Date());
const createdDate = useMemo(() => new Date(annotation.created), [ const createdDate = useMemo(() => new Date(annotationCreated), [
annotation.created, annotationCreated,
]); ]);
const updatedDate = useMemo( const updatedDate = useMemo(
() => withEditedTimestamp && new Date(annotation.updated), () => withEditedTimestamp && new Date(annotationUpdated),
[annotation.updated, withEditedTimestamp] [annotationUpdated, withEditedTimestamp]
); );
const created = useMemo(() => { const created = useMemo(() => {
...@@ -61,10 +65,10 @@ export default function AnnotationTimestamps({ ...@@ -61,10 +65,10 @@ export default function AnnotationTimestamps({
// Determine which of the two Dates to use for the `decayingInterval` // Determine which of the two Dates to use for the `decayingInterval`
// It should be the latest relevant date, as the interval will be // It should be the latest relevant date, as the interval will be
// shorter the closer the date is to "now" // shorter the closer the date is to "now"
const laterDate = updatedDate ? annotation.updated : annotation.created; const laterDate = updatedDate ? annotationUpdated : annotationCreated;
const cancelRefresh = decayingInterval(laterDate, () => setNow(new Date())); const cancelRefresh = decayingInterval(laterDate, () => setNow(new Date()));
return cancelRefresh; return cancelRefresh;
}, [annotation, createdDate, updatedDate, now]); }, [annotationCreated, annotationUpdated, createdDate, updatedDate, now]);
// Do not show the relative timestamp for the edited date if it is the same // Do not show the relative timestamp for the edited date if it is the same
// as the relative timestamp for the created date // as the relative timestamp for the created date
...@@ -73,8 +77,6 @@ export default function AnnotationTimestamps({ ...@@ -73,8 +77,6 @@ export default function AnnotationTimestamps({
? `edited ${updated.relative}` ? `edited ${updated.relative}`
: 'edited'; : 'edited';
const annotationUrl = annotation.links?.html || '';
return ( return (
<div className="AnnotationTimestamps"> <div className="AnnotationTimestamps">
{withEditedTimestamp && ( {withEditedTimestamp && (
...@@ -105,6 +107,8 @@ export default function AnnotationTimestamps({ ...@@ -105,6 +107,8 @@ export default function AnnotationTimestamps({
} }
AnnotationTimestamps.propTypes = { AnnotationTimestamps.propTypes = {
annotation: propTypes.object.isRequired, annotationCreated: propTypes.string,
annotationUpdated: propTypes.string,
annotationUrl: propTypes.string,
withEditedTimestamp: propTypes.bool, withEditedTimestamp: propTypes.bool,
}; };
import { mount } from 'enzyme'; import { mount } from 'enzyme';
import * as fixtures from '../../../test/annotation-fixtures';
import { act } from 'preact/test-utils'; import { act } from 'preact/test-utils';
import { checkAccessibility } from '../../../../test-util/accessibility'; import { checkAccessibility } from '../../../../test-util/accessibility';
...@@ -15,7 +14,9 @@ describe('AnnotationTimestamps', () => { ...@@ -15,7 +14,9 @@ describe('AnnotationTimestamps', () => {
const createComponent = props => const createComponent = props =>
mount( mount(
<AnnotationTimestamps <AnnotationTimestamps
annotation={fixtures.defaultAnnotation()} annotationCreated="2015-05-10T20:18:56.613388+00:00"
annotationUpdated="2015-05-10T20:18:56.613388+00:00"
annotationUrl="http://www.example.com"
withEditedTimestamp={false} withEditedTimestamp={false}
{...props} {...props}
/> />
...@@ -43,10 +44,7 @@ describe('AnnotationTimestamps', () => { ...@@ -43,10 +44,7 @@ describe('AnnotationTimestamps', () => {
}); });
it('renders a linked created timestamp if annotation has a link', () => { it('renders a linked created timestamp if annotation has a link', () => {
const annotation = fixtures.defaultAnnotation(); const wrapper = createComponent();
annotation.links = { html: 'http://www.example.com' };
const wrapper = createComponent({ annotation });
const link = wrapper.find('a.AnnotationTimestamps__created'); const link = wrapper.find('a.AnnotationTimestamps__created');
assert.equal(link.prop('href'), 'http://www.example.com'); assert.equal(link.prop('href'), 'http://www.example.com');
...@@ -55,7 +53,7 @@ describe('AnnotationTimestamps', () => { ...@@ -55,7 +53,7 @@ describe('AnnotationTimestamps', () => {
}); });
it('renders an unlinked created timestamp if annotation does not have a link', () => { it('renders an unlinked created timestamp if annotation does not have a link', () => {
const wrapper = createComponent(); const wrapper = createComponent({ annotationUrl: '' });
const link = wrapper.find('a.AnnotationTimestamps__created'); const link = wrapper.find('a.AnnotationTimestamps__created');
const span = wrapper.find('span.AnnotationTimestamps__created'); const span = wrapper.find('span.AnnotationTimestamps__created');
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment