Commit f43663c6 authored by Robert Knight's avatar Robert Knight

Remove unnecessary array filter

Remove a filter callback which always returned true. The first `!ann.id`
condition is already part of the `store.unsavedAnnotations` logic. The
second `isHighlight(...)` check did not work properly because the input
was an annotation stub object with only `id` and `$tag` fields instead
of the `Annotation` that `isHighlight` expects. For annotation stubs
with no id, it always returned false.

From a functional perspective the `isHighlight` check was also
unnecessary. `store.unsavedAnnotations()` only returns IDs of
annotations which have active drafts, but for highlights no draft is
created.
parent 7bc88d9c
...@@ -2,7 +2,6 @@ import { useEffect, useLayoutEffect, useMemo, useState } from 'preact/hooks'; ...@@ -2,7 +2,6 @@ import { useEffect, useLayoutEffect, useMemo, useState } from 'preact/hooks';
import debounce from 'lodash.debounce'; import debounce from 'lodash.debounce';
import { ListenerCollection } from '../../shared/listener-collection'; import { ListenerCollection } from '../../shared/listener-collection';
import { isHighlight } from '../helpers/annotation-metadata';
import { import {
calculateVisibleThreads, calculateVisibleThreads,
THREAD_DIMENSION_DEFAULTS, THREAD_DIMENSION_DEFAULTS,
...@@ -94,11 +93,7 @@ function ThreadList({ threads }) { ...@@ -94,11 +93,7 @@ function ThreadList({ threads }) {
const newAnnotationTag = (() => { const newAnnotationTag = (() => {
// If multiple unsaved annotations exist, assume that the last one in the // If multiple unsaved annotations exist, assume that the last one in the
// list is the most recently created one. // list is the most recently created one.
const newAnnotations = store const newAnnotations = store.unsavedAnnotations();
.unsavedAnnotations()
// @ts-expect-error - FIXME: The `isHighlight` check here will always fail
// because `unsavedAnnotations` returns objects that do not have a `$highlight` property.
.filter(ann => !ann.id && !isHighlight(ann));
if (!newAnnotations.length) { if (!newAnnotations.length) {
return null; return null;
} }
......
...@@ -9,7 +9,6 @@ import mockImportedComponents from '../../../test-util/mock-imported-components' ...@@ -9,7 +9,6 @@ import mockImportedComponents from '../../../test-util/mock-imported-components'
describe('ThreadList', () => { describe('ThreadList', () => {
let fakeDomUtil; let fakeDomUtil;
let fakeMetadata;
let fakeTopThread; let fakeTopThread;
let fakeScrollContainer; let fakeScrollContainer;
let fakeStore; let fakeStore;
...@@ -32,9 +31,6 @@ describe('ThreadList', () => { ...@@ -32,9 +31,6 @@ describe('ThreadList', () => {
fakeDomUtil = { fakeDomUtil = {
getElementHeightWithMargins: sinon.stub().returns(0), getElementHeightWithMargins: sinon.stub().returns(0),
}; };
fakeMetadata = {
isHighlight: sinon.stub().returns(false),
};
fakeScrollContainer = document.createElement('div'); fakeScrollContainer = document.createElement('div');
fakeScrollContainer.className = 'js-thread-list-scroll-root'; fakeScrollContainer.className = 'js-thread-list-scroll-root';
...@@ -71,7 +67,6 @@ describe('ThreadList', () => { ...@@ -71,7 +67,6 @@ describe('ThreadList', () => {
$imports.$mock(mockImportedComponents()); $imports.$mock(mockImportedComponents());
$imports.$mock({ $imports.$mock({
'../store/use-store': { useStoreProxy: () => fakeStore }, '../store/use-store': { useStoreProxy: () => fakeStore },
'../helpers/annotation-metadata': fakeMetadata,
'../util/dom': fakeDomUtil, '../util/dom': fakeDomUtil,
'../helpers/visible-threads': fakeVisibleThreadsUtil, '../helpers/visible-threads': fakeVisibleThreadsUtil,
}); });
......
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